diff --git a/cmd/proxyctl/maintenance_test.go b/cmd/proxyctl/maintenance_test.go index a10bf71..a2ec729 100644 --- a/cmd/proxyctl/maintenance_test.go +++ b/cmd/proxyctl/maintenance_test.go @@ -4,12 +4,14 @@ import ( "bytes" "context" "errors" + "fmt" "os" "os/exec" "path/filepath" "strings" "testing" + "proxyctl/internal/gnome" "proxyctl/internal/operationlock" "proxyctl/internal/oplog" "proxyctl/internal/profile" @@ -206,6 +208,93 @@ func isolatedProcessEnvironment(root string) []string { ) } +type passwordEchoRunner struct { + values map[string]string + failPasswordSet bool +} + +func newPasswordEchoRunner(savedPassword string) *passwordEchoRunner { + return &passwordEchoRunner{values: map[string]string{ + "org.gnome.system.proxy mode": "'auto'", "org.gnome.system.proxy autoconfig-url": "''", "org.gnome.system.proxy ignore-hosts": "[]", + "org.gnome.system.proxy.http host": "''", "org.gnome.system.proxy.http port": "0", "org.gnome.system.proxy.http use-authentication": "false", + "org.gnome.system.proxy.http authentication-user": "''", "org.gnome.system.proxy.http authentication-password": "'" + savedPassword + "'", + "org.gnome.system.proxy.https host": "''", "org.gnome.system.proxy.https port": "0", "org.gnome.system.proxy.socks host": "''", "org.gnome.system.proxy.socks port": "0", + }} +} + +func (r *passwordEchoRunner) Run(_ context.Context, command gnome.Command) (string, error) { + if len(command.Args) == 1 && command.Args[0] == "list-schemas" { + return "org.gnome.system.proxy\norg.gnome.system.proxy.http\norg.gnome.system.proxy.https\norg.gnome.system.proxy.socks\n", nil + } + if len(command.Args) == 3 && command.Args[0] == "get" { + return r.values[command.Args[1]+" "+command.Args[2]] + "\n", nil + } + if len(command.Args) == 4 && command.Args[0] == "set" { + if r.failPasswordSet && command.Args[2] == "authentication-password" { + return "", fmt.Errorf("runner echoed saved argument %s: %w", command.Args[3], os.ErrPermission) + } + r.values[command.Args[1]+" "+command.Args[2]] = command.Args[3] + return "", nil + } + return "", fmt.Errorf("unexpected command: %v", command.Args) +} + +func passwordRestoreFixture(t *testing.T, secret string) (*gnome.Adapter, *passwordEchoRunner) { + t.Helper() + root := t.TempDir() + runner := newPasswordEchoRunner(secret) + statePath := filepath.Join(root, "state", "gnome.json") + adapter := gnome.New(gnome.Configuration{CommandName: "gsettings", UID: os.Getuid(), GID: os.Getgid(), HomeDir: root, StateFile: statePath}, runner, gnome.NewFileState(statePath, -1, -1, root)) + active := &profile.Profile{Name: "office", Mode: profile.AdvancedMode, HTTPURL: "http://proxy.example:8080", Bypass: []string{"localhost"}} + if result := adapter.SetDesired(context.Background(), true, active); result.Err != nil || result.Inspection.State != target.Enabled { + t.Fatalf("prepare managed GNOME Target: %#v", result) + } + runner.failPasswordSet = true + return adapter, runner +} + +func TestResetSuppressesSavedGNOMEPasswordFromOperationCLIAndLogsWithoutActiveProfile(t *testing.T) { + const secret = "GNOME-ORIGINAL-PASSWORD-FULL-CHAIN-07" + + direct, _ := passwordRestoreFixture(t, secret) + operation := direct.SetDesired(context.Background(), false, nil) + if operation.Err == nil { + t.Fatal("password restore unexpectedly succeeded") + } + if !errors.Is(operation.Err, os.ErrPermission) { + t.Fatalf("OperationResult lost safe permission classification: %v", operation.Err) + } + if rendered := fmt.Sprint(operation); strings.Contains(rendered, secret) || strings.Contains(rendered, "runner echoed") { + t.Fatalf("unsafe OperationResult: %s", rendered) + } + + managed, _ := passwordRestoreFixture(t, secret) + root := t.TempDir() + config := filepath.Join(root, "config", "profiles.json") + writeConfig(t, config, `{"profiles":[]}`) + store := profile.NewFileStore(config) + stateDir := filepath.Join(root, "state") + lock := operationlock.New(filepath.Join(stateDir, "operation.lock")) + logger := oplog.New(filepath.Join(stateDir, "operations.log"), 1024, 1) + var output bytes.Buffer + err := runMaintenanceCommand([]string{"reset"}, strings.NewReader("yes\n"), &output, store, []target.Target{managed}, lock, logger, nil) + if err == nil { + t.Fatal("reset unexpectedly succeeded") + } + logData, readErr := os.ReadFile(filepath.Join(stateDir, "operations.log")) + if readErr != nil { + t.Fatal(readErr) + } + for surface, value := range map[string]string{"CLI output": output.String(), "returned error": err.Error(), "operation log": string(logData)} { + if strings.Contains(value, secret) || strings.Contains(value, "runner echoed") { + t.Fatalf("%s exposed saved password or raw runner error: %q", surface, value) + } + if !strings.Contains(value, "sensitive argument and runner error suppressed") { + t.Fatalf("%s lacks fixed safe failure message: %q", surface, value) + } + } +} + func TestResetCommandCancellationAndTruthfulPartialFailureWithFakeTargets(t *testing.T) { root := t.TempDir() config := filepath.Join(root, "xdg-config", "proxyctl", "profiles.json") diff --git a/internal/dockerdaemon/dockerdaemon.go b/internal/dockerdaemon/dockerdaemon.go index c5471a4..ea84e18 100644 --- a/internal/dockerdaemon/dockerdaemon.go +++ b/internal/dockerdaemon/dockerdaemon.go @@ -20,7 +20,10 @@ import ( "proxyctl/internal/target" ) -const service = "docker.service" +const ( + service = "docker.service" + recoveryVersion = 1 +) type Configuration struct { DaemonConfigPath string @@ -188,9 +191,10 @@ func parseProperties(output string) map[string]string { } type recovery struct { + Version int `json:"version"` Original []byte `json:"original,omitempty"` OriginalExists bool `json:"originalExists"` - OriginalMode uint32 `json:"originalMode,omitempty"` + OriginalMode uint32 `json:"originalMode"` ConfigFingerprint string `json:"configFingerprint"` RestartedFingerprint string `json:"restartedFingerprint,omitempty"` } @@ -277,6 +281,7 @@ func (a *Adapter) enable(ctx context.Context, p profile.Profile) error { return fmt.Errorf("read Docker daemon configuration: %w", readErr) } if !managed { + metadata.Version = recoveryVersion metadata.OriginalExists = readErr == nil if metadata.OriginalExists { metadata.Original = append([]byte(nil), current...) @@ -372,12 +377,59 @@ func (a *Adapter) readRecovery() (recovery, bool, error) { if err != nil { return recovery{}, false, err } + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + return recovery{}, true, err + } + if fields == nil { + return recovery{}, true, errors.New("Docker daemon recovery metadata must be a JSON object") + } var value recovery if err := json.Unmarshal(data, &value); err != nil { return recovery{}, true, err } + if err := validateRecovery(value, fields); err != nil { + return recovery{}, true, err + } return value, true, nil } + +func validateRecovery(value recovery, fields map[string]json.RawMessage) error { + for _, name := range []string{"version", "originalExists", "originalMode", "configFingerprint"} { + if _, present := fields[name]; !present { + return fmt.Errorf("Docker daemon recovery metadata is missing required field %q", name) + } + } + if value.Version != recoveryVersion { + return fmt.Errorf("unsupported Docker daemon recovery metadata version %d", value.Version) + } + if !validFingerprint(value.ConfigFingerprint) { + return errors.New("Docker daemon recovery metadata has an invalid configuration fingerprint") + } + if value.RestartedFingerprint != "" && (value.RestartedFingerprint != value.ConfigFingerprint || !validFingerprint(value.RestartedFingerprint)) { + return errors.New("Docker daemon recovery metadata has an inconsistent restarted fingerprint") + } + _, hasOriginal := fields["original"] + if value.OriginalExists { + if !hasOriginal { + return errors.New("Docker daemon recovery metadata is missing the recorded original configuration or mode") + } + if value.OriginalMode > 0o777 { + return errors.New("Docker daemon recovery metadata has an invalid original file mode") + } + if err := validateConfigSchema(value.Original); err != nil { + return fmt.Errorf("Docker daemon recovery metadata has an invalid original configuration: %w", err) + } + } else if hasOriginal || len(value.Original) != 0 || value.OriginalMode != 0 { + return errors.New("Docker daemon recovery metadata records original data although the original did not exist") + } + return nil +} + +func validFingerprint(value string) bool { + decoded, err := hex.DecodeString(value) + return err == nil && len(decoded) == sha256.Size +} func (a *Adapter) writeRecovery(value recovery) error { data, err := json.MarshalIndent(value, "", " ") if err != nil { diff --git a/internal/dockerdaemon/dockerdaemon_test.go b/internal/dockerdaemon/dockerdaemon_test.go index 94af2ba..f07448a 100644 --- a/internal/dockerdaemon/dockerdaemon_test.go +++ b/internal/dockerdaemon/dockerdaemon_test.go @@ -11,6 +11,7 @@ import ( "strings" "testing" + "proxyctl/internal/app" "proxyctl/internal/dockerdaemon" "proxyctl/internal/profile" "proxyctl/internal/target" @@ -106,6 +107,61 @@ type fixedAuthority int func (a fixedAuthority) EffectiveUID() int { return int(a) } +type immediateLock struct{} + +func (immediateLock) Lock(context.Context) (func(), error) { return func() {}, nil } + +func TestUnsafeRecoveryMetadataIsDriftedAndResetNeverMutatesDaemon(t *testing.T) { + fingerprint := strings.Repeat("a", 64) + cases := map[string]string{ + "empty object": `{}`, + "truncated": `{"version":1`, + "unsupported version": `{"version":2,"originalExists":false,"configFingerprint":"` + fingerprint + `"}`, + "missing original": `{"version":1,"originalExists":true,"originalMode":384,"configFingerprint":"` + fingerprint + `"}`, + "unexpected original": `{"version":1,"originalExists":false,"original":"e30=","configFingerprint":"` + fingerprint + `"}`, + "mode without original": `{"version":1,"originalExists":false,"originalMode":384,"configFingerprint":"` + fingerprint + `"}`, + "invalid mode": `{"version":1,"originalExists":true,"original":"e30=","originalMode":4096,"configFingerprint":"` + fingerprint + `"}`, + "invalid fingerprint": `{"version":1,"originalExists":false,"configFingerprint":"short"}`, + "contradictory restart record": `{"version":1,"originalExists":false,"configFingerprint":"` + fingerprint + `","restartedFingerprint":"` + strings.Repeat("b", 64) + `"}`, + } + for name, metadata := range cases { + t.Run(name, func(t *testing.T) { + a, record, daemonPath, recoveryPath, _ := fixture(t) + originalConfig := []byte(`{"log-level":"debug"}`) + if err := os.MkdirAll(filepath.Dir(daemonPath), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(daemonPath, originalConfig, 0o640); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(recoveryPath), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(recoveryPath, []byte(metadata), 0o600); err != nil { + t.Fatal(err) + } + record.events = nil + + if inspection := a.Inspect(context.Background(), nil); inspection.State != target.Drifted { + t.Fatalf("inspection = %#v", inspection) + } + reset := app.NewMaintenance(profile.State{}, []target.Target{a}, immediateLock{}, nil).Reset(context.Background(), true) + if reset.Err == nil || len(reset.Targets) != 1 || reset.Targets[0].Err == nil { + t.Fatalf("unsafe reset = %#v", reset) + } + if events := mutationEvents(record.events); len(events) != 0 { + t.Fatalf("unsafe metadata caused mutation: %#v", events) + } + if got, err := os.ReadFile(daemonPath); err != nil || !reflect.DeepEqual(got, originalConfig) { + t.Fatalf("daemon changed to %q: %v", got, err) + } + if got, err := os.ReadFile(recoveryPath); err != nil || string(got) != metadata { + t.Fatalf("recovery changed to %q: %v", got, err) + } + }) + } +} + func TestDiagnosticChecksCoverSystemdSchemaAndAuthorityWithoutMutation(t *testing.T) { a, record, daemonPath, recoveryPath, runner := fixture(t) a = dockerdaemon.New(dockerdaemon.Configuration{DaemonConfigPath: daemonPath, RecoveryPath: recoveryPath, TrustedRoot: filepath.Dir(filepath.Dir(daemonPath))}, recordingFS{recorder: record}, runner, fixedAuthority(0)) diff --git a/internal/gnome/gnome.go b/internal/gnome/gnome.go index 297faa8..dab8d96 100644 --- a/internal/gnome/gnome.go +++ b/internal/gnome/gnome.go @@ -26,10 +26,11 @@ import ( ) const ( - proxySchema = "org.gnome.system.proxy" - httpSchema = proxySchema + ".http" - httpsSchema = proxySchema + ".https" - socksSchema = proxySchema + ".socks" + gnomeRecoveryVersion = 1 + proxySchema = "org.gnome.system.proxy" + httpSchema = proxySchema + ".http" + httpsSchema = proxySchema + ".https" + socksSchema = proxySchema + ".socks" ) var requiredSchemas = []string{proxySchema, httpSchema, httpsSchema, socksSchema} @@ -226,6 +227,7 @@ type settings struct { } type recovery struct { + Version int `json:"version"` Original settings `json:"original"` LastFingerprint string `json:"lastFingerprint"` } @@ -421,6 +423,7 @@ func (a *Adapter) enable(ctx context.Context, active profile.Profile) error { return fmt.Errorf("read GNOME recovery metadata: %w", err) } if !managed { + metadata.Version = gnomeRecoveryVersion original, err := a.readSettings(ctx) if err != nil { return fmt.Errorf("snapshot original GNOME settings: %w", err) @@ -480,13 +483,59 @@ func (a *Adapter) readRecovery() (recovery, bool, error) { if err != nil { return recovery{}, false, err } + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + return recovery{}, true, err + } + if fields == nil { + return recovery{}, true, errors.New("GNOME recovery metadata must be a JSON object") + } + for _, name := range []string{"version", "original", "lastFingerprint"} { + if _, present := fields[name]; !present { + return recovery{}, true, fmt.Errorf("GNOME recovery metadata is missing required field %q", name) + } + } + var originalFields map[string]json.RawMessage + if err := json.Unmarshal(fields["original"], &originalFields); err != nil || originalFields == nil { + return recovery{}, true, errors.New("GNOME recovery metadata original settings must be a JSON object") + } + for _, name := range []string{"mode", "autoConfigUrl", "ignoreHosts", "httpHost", "httpPort", "httpUseAuthentication", "httpAuthenticationUser", "httpAuthenticationPassword", "httpsHost", "httpsPort", "socksHost", "socksPort"} { + if _, present := originalFields[name]; !present { + return recovery{}, true, fmt.Errorf("GNOME recovery metadata original settings are missing required field %q", name) + } + } var value recovery if err := json.Unmarshal(data, &value); err != nil { return recovery{}, true, err } + if err := validateRecovery(value); err != nil { + return recovery{}, true, err + } return value, true, nil } +func validateRecovery(value recovery) error { + if value.Version != gnomeRecoveryVersion { + return fmt.Errorf("unsupported GNOME recovery metadata version %d", value.Version) + } + decoded, err := hex.DecodeString(value.LastFingerprint) + if err != nil || len(decoded) != sha256.Size { + return errors.New("GNOME recovery metadata has an invalid fingerprint") + } + if value.Original.Mode != "none" && value.Original.Mode != "manual" && value.Original.Mode != "auto" { + return errors.New("GNOME recovery metadata has an invalid original proxy mode") + } + if value.Original.IgnoreHosts == nil { + return errors.New("GNOME recovery metadata has an incomplete original ignore-hosts snapshot") + } + for _, port := range []int{value.Original.HTTPPort, value.Original.HTTPSPort, value.Original.SocksPort} { + if port < 0 || port > 65535 { + return errors.New("GNOME recovery metadata has an invalid original proxy port") + } + } + return nil +} + func (a *Adapter) command(args ...string) Command { return Command{Name: a.configuration.CommandName, Args: append([]string(nil), args...), UID: a.configuration.UID, GID: a.configuration.GID, Env: map[string]string{ "HOME": a.configuration.HomeDir, "USER": a.configuration.Username, "LOGNAME": a.configuration.Username, @@ -511,6 +560,13 @@ func (a *Adapter) set(ctx context.Context, schema, key, value string) error { return nil } +func (a *Adapter) setSensitiveString(ctx context.Context, schema, key, value string) error { + if _, err := a.run(ctx, "set", schema, key, value); err != nil { + return safeRedaction("set GNOME authentication password failed; sensitive argument and runner error suppressed", err) + } + return nil +} + func (a *Adapter) readSettings(ctx context.Context) (settings, error) { var result settings var err error @@ -615,7 +671,7 @@ func (a *Adapter) writeSettings(ctx context.Context, value settings) error { {proxySchema, "autoconfig-url", quote(value.AutoConfigURL)}, {proxySchema, "ignore-hosts", quoteStrings(value.IgnoreHosts)}, {httpSchema, "host", quote(value.HTTPHost)}, {httpSchema, "port", strconv.Itoa(value.HTTPPort)}, - {httpSchema, "use-authentication", strconv.FormatBool(value.HTTPUseAuth)}, {httpSchema, "authentication-user", quote(value.HTTPAuthUser)}, {httpSchema, "authentication-password", quote(value.HTTPAuthPassword)}, + {httpSchema, "use-authentication", strconv.FormatBool(value.HTTPUseAuth)}, {httpSchema, "authentication-user", quote(value.HTTPAuthUser)}, {httpsSchema, "host", quote(value.HTTPSHost)}, {httpsSchema, "port", strconv.Itoa(value.HTTPSPort)}, {socksSchema, "host", quote(value.SocksHost)}, {socksSchema, "port", strconv.Itoa(value.SocksPort)}, } @@ -624,6 +680,9 @@ func (a *Adapter) writeSettings(ctx context.Context, value settings) error { return err } } + if err := a.setSensitiveString(ctx, httpSchema, "authentication-password", quote(value.HTTPAuthPassword)); err != nil { + return err + } return a.set(ctx, proxySchema, "mode", quote(value.Mode)) } diff --git a/internal/gnome/gnome_test.go b/internal/gnome/gnome_test.go index 8638e2f..cc05d64 100644 --- a/internal/gnome/gnome_test.go +++ b/internal/gnome/gnome_test.go @@ -108,6 +108,91 @@ func active() *profile.Profile { return &profile.Profile{Name: "office", Mode: profile.AdvancedMode, HTTPURL: "http://proxy.example:8080", HTTPSURL: "http://secure.example:8443", AllURL: "socks5://socks.example:1080", Bypass: []string{"localhost", ".example.com", "10.0.0.0/8", "2001:db8::1"}} } +func TestAuthenticationPasswordSetFailureSuppressesRunnerErrorAndProfileArgument(t *testing.T) { + const profilePassword = "GNOME-PROFILE-PASSWORD-07" + r, state := newRunner(), &memoryState{} + r.fail = func(c gnome.Command) error { + if len(c.Args) == 4 && c.Args[0] == "set" && c.Args[2] == "authentication-password" { + return fmt.Errorf("runner echoed argument %s: %w", c.Args[3], os.ErrPermission) + } + return nil + } + p := *active() + p.HTTPURL = "http://alice:" + profilePassword + "@proxy.example:8080" + result := adapter(r, state).SetDesired(context.Background(), true, &p) + if result.Err == nil || !errors.Is(result.Err, os.ErrPermission) { + t.Fatalf("set error classification = %#v", result) + } + if strings.Contains(result.Err.Error(), profilePassword) || strings.Contains(result.Err.Error(), "runner echoed") { + t.Fatalf("set exposed runner error or password argument: %v", result.Err) + } + if !strings.Contains(result.Err.Error(), "sensitive argument and runner error suppressed") { + t.Fatalf("set lacks fixed safe message: %v", result.Err) + } +} + +func TestAuthenticationPasswordRestoreFailureSuppressesRunnerErrorAndSavedArgument(t *testing.T) { + const savedPassword = "GNOME-SAVED-ORIGINAL-PASSWORD-07" + r, state := newRunner(), &memoryState{} + r.values["org.gnome.system.proxy mode"] = "'auto'" + r.values["org.gnome.system.proxy.http authentication-password"] = "'" + savedPassword + "'" + a := adapter(r, state) + if result := a.SetDesired(context.Background(), true, active()); result.Err != nil { + t.Fatalf("enable = %#v", result) + } + r.fail = func(c gnome.Command) error { + if reflect.DeepEqual(c.Args[:min(len(c.Args), 3)], []string{"set", "org.gnome.system.proxy.http", "authentication-password"}) { + return fmt.Errorf("runner echoed argument %s: %w", c.Args[3], os.ErrPermission) + } + return nil + } + result := a.SetDesired(context.Background(), false, nil) + if result.Err == nil || !errors.Is(result.Err, os.ErrPermission) { + t.Fatalf("restore error classification = %#v", result) + } + if strings.Contains(result.Err.Error(), savedPassword) || strings.Contains(result.Err.Error(), "runner echoed") { + t.Fatalf("restore exposed runner error or password argument: %v", result.Err) + } + if !strings.Contains(result.Err.Error(), "sensitive argument and runner error suppressed") { + t.Fatalf("restore lacks fixed safe message: %v", result.Err) + } +} + +func TestIncompleteRecoveryMetadataRejectsMutationAndPreservesGNOMESettings(t *testing.T) { + cases := map[string]string{ + "empty object": `{}`, + "missing original": `{"version":1,"lastFingerprint":"` + strings.Repeat("a", 64) + `"}`, + "incomplete original": `{"version":1,"lastFingerprint":"` + strings.Repeat("a", 64) + `","original":{"mode":"none"}}`, + "invalid fingerprint": `{"version":1,"lastFingerprint":"short","original":{}}`, + } + for name, metadata := range cases { + t.Run(name, func(t *testing.T) { + r, state := newRunner(), &memoryState{data: []byte(metadata)} + before := make(map[string]string, len(r.values)) + for key, value := range r.values { + before[key] = value + } + a := adapter(r, state) + if inspection := a.Inspect(context.Background(), nil); inspection.State != target.Drifted { + t.Fatalf("inspection = %#v", inspection) + } + callStart := len(r.calls) + result := a.SetDesired(context.Background(), false, nil) + if result.Err == nil || result.Inspection.State != target.Drifted { + t.Fatalf("unsafe disable = %#v", result) + } + if !reflect.DeepEqual(r.values, before) || string(state.data) != metadata { + t.Fatalf("configuration mutated: values=%#v metadata=%q", r.values, state.data) + } + for _, call := range r.calls[callStart:] { + if len(call.Args) > 0 && call.Args[0] == "set" { + t.Fatalf("unsafe metadata caused set: %#v", call) + } + } + }) + } +} + func TestDiagnosticsSuppressSensitiveGSettingsOutputAndRunnerErrors(t *testing.T) { const secret = "GNOME-DIAGNOSTIC-SECRET-7f3a" for _, test := range []struct { diff --git a/internal/shell/shell.go b/internal/shell/shell.go index 350e803..f01eb46 100644 --- a/internal/shell/shell.go +++ b/internal/shell/shell.go @@ -22,8 +22,9 @@ import ( ) const ( - integrationStart = "# >>> proxyctl shell integration >>>" - integrationEnd = "# <<< proxyctl shell integration <<<" + integrationStart = "# >>> proxyctl shell integration >>>" + integrationEnd = "# <<< proxyctl shell integration <<<" + shellRecoveryVersion = 1 ) var proxyVariables = []string{ @@ -149,7 +150,8 @@ func (f OSFileSystem) AtomicWrite(path string, data []byte, mode os.FileMode, ow } type recovery struct { - Original map[string]string `json:"original,omitempty"` + Version int `json:"version"` + Original map[string]string `json:"original"` OriginalScript *string `json:"originalScript,omitempty"` Fingerprint string `json:"fingerprint"` } @@ -288,6 +290,7 @@ func (a *Adapter) enable(active profile.Profile) error { return fmt.Errorf("read Shell recovery metadata: %w", err) } if !managed { + metadata.Version = shellRecoveryVersion if existing, readErr := a.files.ReadFile(a.configuration.EnvironmentFile); readErr == nil && recognizesProxy(existing) { original := string(existing) metadata.OriginalScript = &original @@ -344,12 +347,57 @@ func (a *Adapter) readRecovery() (recovery, bool, error) { if err != nil { return recovery{}, false, err } + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + return recovery{}, true, err + } + if fields == nil { + return recovery{}, true, errors.New("Shell recovery metadata must be a JSON object") + } var value recovery if err := json.Unmarshal(data, &value); err != nil { return recovery{}, true, err } + if err := validateRecovery(value, fields); err != nil { + return recovery{}, true, err + } return value, true, nil } + +func validateRecovery(value recovery, fields map[string]json.RawMessage) error { + for _, name := range []string{"version", "original", "fingerprint"} { + if _, present := fields[name]; !present { + return fmt.Errorf("Shell recovery metadata is missing required field %q", name) + } + } + if value.Version != shellRecoveryVersion { + return fmt.Errorf("unsupported Shell recovery metadata version %d", value.Version) + } + decoded, err := hex.DecodeString(value.Fingerprint) + if err != nil || len(decoded) != sha256.Size { + return errors.New("Shell recovery metadata has an invalid fingerprint") + } + _, hasOriginalScript := fields["originalScript"] + if hasOriginalScript { + if value.OriginalScript == nil || value.Original != nil || len(*value.OriginalScript) == 0 || !recognizesProxy([]byte(*value.OriginalScript)) { + return errors.New("Shell recovery metadata has inconsistent original script state") + } + return nil + } + if value.Original == nil { + return errors.New("Shell recovery metadata is missing the original environment snapshot") + } + for name := range value.Original { + known := false + for _, candidate := range proxyVariables { + known = known || name == candidate + } + if !known { + return fmt.Errorf("Shell recovery metadata contains unsupported variable %q", name) + } + } + return nil +} func (a *Adapter) ownership() *Ownership { if !a.configuration.OwnFiles { return nil diff --git a/internal/shell/shell_test.go b/internal/shell/shell_test.go index bb9313e..9dfbc84 100644 --- a/internal/shell/shell_test.go +++ b/internal/shell/shell_test.go @@ -77,6 +77,43 @@ func TestShellAdapterRunsReusableTargetContract(t *testing.T) { } } +func TestIncompleteRecoveryMetadataRejectsRestoreWithoutChangingShellConfiguration(t *testing.T) { + fingerprint := strings.Repeat("a", 64) + cases := map[string]string{ + "empty object": `{}`, + "missing restoration": `{"version":1,"fingerprint":"` + fingerprint + `","original":null}`, + "invalid fingerprint": `{"version":1,"fingerprint":"short","original":{}}`, + "conflicting originals": `{"version":1,"fingerprint":"` + fingerprint + `","original":{"HTTP_PROXY":"http://prior.example:3128"},"originalScript":"export HTTP_PROXY=http://prior.example:3128\\n"}`, + } + for name, metadata := range cases { + t.Run(name, func(t *testing.T) { + a := newAdapter(t, "/bin/bash", nil) + environment := []byte("export HTTP_PROXY='http://managed.example:8080'\n") + for path, data := range map[string][]byte{a.EnvironmentPath(): environment, a.RecoveryPath(): []byte(metadata)} { + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0o600); err != nil { + t.Fatal(err) + } + } + if inspection := a.Inspect(context.Background(), nil); inspection.State != target.Drifted { + t.Fatalf("inspection = %#v", inspection) + } + result := a.SetDesired(context.Background(), false, nil) + if result.Err == nil || result.Inspection.State != target.Drifted { + t.Fatalf("unsafe disable = %#v", result) + } + if got := readFile(t, a.EnvironmentPath()); got != string(environment) { + t.Fatalf("environment changed to %q", got) + } + if got := readFile(t, a.RecoveryPath()); got != metadata { + t.Fatalf("recovery changed to %q", got) + } + }) + } +} + func TestRecoveryFailureRetryPreservesOriginalShellState(t *testing.T) { files := &failingFS{FileSystem: shell.OSFileSystem{}, failRecoveryOnce: true} a := newAdapterWithFS(t, "/bin/bash", map[string]string{"HTTP_PROXY": "http://prior.example:3128"}, files)