diff --git a/cmd/rvbox/main.go b/cmd/rvbox/main.go index 9c87f74..f60176e 100644 --- a/cmd/rvbox/main.go +++ b/cmd/rvbox/main.go @@ -216,6 +216,11 @@ func runClientDaemon(ctx context.Context, configPath string, diagnostics io.Writ Jitter: agent.CryptoJitter, Now: func() time.Time { return time.Now().UTC() }, OnDispatch: executor.Dispatch, OnScriptReady: executor.ScriptReady, OnStdin: executor.Stdin, OnCloseStdin: executor.CloseStdin, OnSignal: executor.Signal, OnTerminate: executor.Terminate, EventReady: eventReady, + OnSessionError: func(sessionErr error) { + if diagnostics != nil { + _, _ = fmt.Fprintf(diagnostics, "rvbox client session retry: %v\n", sessionErr) + } + }, }); runErr != nil && ctx.Err() == nil && diagnostics != nil { _, _ = fmt.Fprintf(diagnostics, "rvbox client session stopped: %v\n", runErr) } diff --git a/cmd/rvbox/service_windows.go b/cmd/rvbox/service_windows.go index 90c364a..218a58b 100644 --- a/cmd/rvbox/service_windows.go +++ b/cmd/rvbox/service_windows.go @@ -14,6 +14,7 @@ import ( clientwindows "github.com/rvbox/rvbox/internal/client/supervisor/windows" "github.com/rvbox/rvbox/internal/client/windowsservice" "github.com/rvbox/rvbox/internal/client/windowstray" + "golang.org/x/sys/windows" "golang.org/x/sys/windows/svc" ) @@ -90,6 +91,16 @@ func openServiceDiagnostics(configPath string, diagnostics io.Writer) (io.Writer } return diagnostics, func() {} } + // Service diagnostics are intentionally not private spool data. Preserve a + // protected SYSTEM/Administrators DACL so an operator can diagnose an SCM + // startup or reconnect failure without weakening access to command state. + if err := applyServiceDiagnosticsACL(path); err != nil { + _ = file.Close() + if diagnostics == nil { + return io.Discard, func() {} + } + return diagnostics, func() {} + } if diagnostics == nil { return file, func() { _ = file.Close() } } @@ -99,6 +110,18 @@ func openServiceDiagnostics(configPath string, diagnostics io.Writer) (io.Writer return io.MultiWriter(file, diagnostics), func() { _ = file.Close() } } +func applyServiceDiagnosticsACL(path string) error { + descriptor, err := windows.SecurityDescriptorFromString("D:P(A;;FA;;;SY)(A;;FA;;;BA)") + if err != nil { + return err + } + dacl, _, err := descriptor.DACL() + if err != nil { + return err + } + return windows.SetNamedSecurityInfo(path, windows.SE_FILE_OBJECT, windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION, nil, nil, dacl, nil) +} + func runTray(configPath string, diagnostics io.Writer) error { _ = configPath // the tray obtains the canonical paths from the service. return windowstray.Run(context.Background(), diagnostics) diff --git a/docs/testing.md b/docs/testing.md index 45bbd89..62eaf35 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -70,8 +70,12 @@ default). Helium hosts the VM only; it does not host any RVBox server containers. The self-signed server certificate is intentionally accepted by the v1 client without a test CA. It then drives the installed SCM service through the server's real Unix control socket and verifies every Windows -execution context. The tagged binary's controlled pre-launch failures are -limited to the test fixture; a release binary rejects that switch. +execution context, ordered stdin close, TERM delivery, and a server-process +restart while a command is running. The restart check preserves the server +state volume, waits for a new reconciled WSS session, then proves that the same +command can receive its terminal signal; it covers reconnect without treating +the old session as valid. The tagged binary's controlled pre-launch failures +are limited to the test fixture; a release binary rejects that switch. Successful runs collect bounded artifacts, remove only their labeled Compose project, and restore the exact clean snapshot. A failed or --keep run stays @@ -222,7 +226,7 @@ that mode-600 file; the controller never puts it on a command line, manifest, log, or artifact. The native lifecycle is `status`, `prepare`, `stage`, `install`, `run`, -`collect`, `stop`, and `reset`. `prepare` verifies the VM and snapshot UUIDs, +`collect`, `logs`, `stop`, and `reset`. `prepare` verifies the VM and snapshot UUIDs, restores the clean baseline, starts headless, waits for Guest Additions, and proves that `RVBoxClient` is absent. `stage` copies a versioned non-secret test bundle through a run-specific host directory to a run-specific guest directory. @@ -230,8 +234,11 @@ bundle through a run-specific host directory to a run-specific guest directory. the real `rvbox.exe --install-service` path and proves completion through SCM. `run` is for reconfiguration/restart scenarios after that first installation. Neither action invokes the GUI-subsystem executable directly with the normal -Guest Control account. `collect` obtains only bounded/redacted artifacts, and -`reset` restores the exact clean baseline and leaves the VM powered off. It +Guest Control account. `collect` obtains only bounded/redacted artifacts. +`logs` is the narrow read-only service-startup/client-diagnostics action for a +retained prepared run. The service diagnostic file grants access to SYSTEM and +local Administrators only; it contains no command spool data. `reset` restores +the exact clean baseline and leaves the VM powered off. It first permits a bounded ACPI shutdown; if that hangs, it force-powers off only the exact leased disposable VM before snapshot restoration. That intentional state loss is confined to the test isolation boundary. diff --git a/internal/client/agent/runner.go b/internal/client/agent/runner.go index 46e25d6..f56054e 100644 --- a/internal/client/agent/runner.go +++ b/internal/client/agent/runner.go @@ -35,6 +35,10 @@ type RunnerOptions struct { OnSignal func(context.Context, Session, *rvboxv1.SignalCommand) error OnScriptReady func(context.Context, Session, domain.UUID) error OnTerminate func(context.Context, domain.UUID) error + // OnSessionError observes one failed dial, handshake, protocol, or active + // transport session before normal reconnect backoff. It must not block; the + // durable spool and retry policy remain owned by Run. + OnSessionError func(error) // EventReady wakes the active session after a supervisor worker appends a // durable event. The network loop remains the sole writer; a reconnect can // safely ignore a stale notification because replay reads the spool again. @@ -95,10 +99,13 @@ func Run(ctx context.Context, options RunnerOptions) error { return nil } sessionStarted := options.Now() - _ = runOnce(ctx, options) + sessionErr := runOnce(ctx, options) if ctx.Err() != nil { return nil } + if sessionErr != nil && options.OnSessionError != nil { + options.OnSessionError(sessionErr) + } if options.Now().Sub(sessionStarted) >= options.Backoff.StableReset { failures = 0 } @@ -344,17 +351,17 @@ func serveActive(ctx context.Context, transport Transport, options RunnerOptions switch { case envelope.GetCommandDispatch() != nil: if err := handleDispatch(ctx, transport, options, session, envelope.GetCommandDispatch(), limits); err != nil { - return err + return fmt.Errorf("handle command dispatch %s: %w", envelope.GetCommandDispatch().GetIssueUuid(), err) } issue, parseErr := domain.ParseUUIDv7(envelope.GetCommandDispatch().GetIssueUuid()) if parseErr == nil { if err := flushEvents(ctx, transport, options.Store, session, issue, limits, sent); err != nil { - return err + return fmt.Errorf("flush dispatched command %s events: %w", issue, err) } } case envelope.GetEventAck() != nil: if err := ApplyEventAck(ctx, options.Store, envelope.GetEventAck()); err != nil { - return err + return fmt.Errorf("apply event acknowledgement for %s: %w", envelope.GetEventAck().GetIssueUuid(), err) } case envelope.GetScriptChunk() != nil: if err := handleScriptChunk(ctx, transport, options.Store, session, envelope.GetScriptChunk(), limits, options.Now, sent); err != nil { diff --git a/internal/client/agent/runner_test.go b/internal/client/agent/runner_test.go index 60f9ca7..1cdd243 100644 --- a/internal/client/agent/runner_test.go +++ b/internal/client/agent/runner_test.go @@ -72,6 +72,36 @@ func TestRunnerOptionsRejectMissingJitter_HP_RUNTIME_02(t *testing.T) { } } +func TestRunReportsRetryableSessionError_HP_RUNTIME_04(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + store, err := spool.Open(ctx, spool.Options{DataDir: filepath.Join(t.TempDir(), "spool"), BusyTimeout: time.Second}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = store.Close() }) + want := errors.New("dial refused") + reported := make(chan error, 1) + hello := &rvboxv1.ClientHello{ClientId: "runner-client", SupportedProtocol: &rvboxv1.ProtocolRange{Major: 1, MinMinor: 0, MaxMinor: 0}, DaemonVersion: "test", Platform: rvboxv1.Platform_PLATFORM_WINDOWS, Architecture: "amd64", DaemonCwd: `C:\\`, SupportedShells: []rvboxv1.ShellType{rvboxv1.ShellType_SHELL_POWERSHELL}, ClientInstanceId: store.ClientInstanceID().String(), MaxRunningCommands: 1, MaxQueuedCommands: 1, SentAt: timestamppb.Now()} + err = Run(ctx, RunnerOptions{ + Store: store, Dial: func(context.Context) (Transport, error) { return nil, want }, Hello: hello, + Limits: agentproto.DefaultLimits(), Backoff: BackoffOptions{Initial: time.Millisecond, Maximum: time.Millisecond, StableReset: time.Second}, + Jitter: func(time.Duration) time.Duration { return 0 }, Now: func() time.Time { return time.Now().UTC() }, + OnSessionError: func(got error) { reported <- got; cancel() }, + }) + if err != nil { + t.Fatalf("Run error = %v, want graceful cancellation", err) + } + select { + case got := <-reported: + if !errors.Is(got, want) { + t.Fatalf("reported error = %v, want %v", got, want) + } + default: + t.Fatal("retryable session error was not reported") + } +} + func TestFlushEventsAssignsAndSendsOnlyUnacknowledgedRows_HP_RUNTIME_03(t *testing.T) { ctx := context.Background() store, err := spool.Open(ctx, spool.Options{DataDir: filepath.Join(t.TempDir(), "spool"), BusyTimeout: time.Second}) diff --git a/scripts/windows/native-test b/scripts/windows/native-test index fc2a017..237a9d7 100755 --- a/scripts/windows/native-test +++ b/scripts/windows/native-test @@ -78,6 +78,15 @@ compose() { docker compose -p "$project" -f "$repo_root/test/linux-server/compose.yaml" "$@" } +remove_project_networks() { + # The project label is Docker Compose's exact resource-ownership key. This + # also reclaims legacy certgen default networks left by fixtures created + # before certgen joined the labeled native network. + for network_id in $(docker network ls --filter "label=com.docker.compose.project=$project" --quiet); do + docker network rm "$network_id" >/dev/null || fail "could not remove owned Docker network $network_id" + done +} + rvc() { compose exec -T server /opt/rvbox/rvc --socket /run/rvbox/server.sock "$@" } @@ -205,6 +214,43 @@ assert_signal_term() { fail "TERM command did not reach terminal state" } +assert_server_restart_reconnect() { + issued=$(rvc run --background --shell cmd "$client_id" 'ping -t 127.0.0.1 >NUL' 2>&1) || fail "restart command admission failed: $issued" + issue=$(printf '%s\n' "$issued" | awk 'NR == 1 { print $1 }') + case $issue in ????????-????-7???-????-????????????) ;; *) fail "restart command returned invalid issue UUID: $issued" ;; esac + attempt=0 + while [ "$attempt" -lt 30 ]; do + result=$(rvc stat "$client_id" "$issue" 2>/dev/null || true) + if printf '%s\n' "$result" | grep -q 'lifecycle=COMMAND_RUNNING'; then break; fi + attempt=$((attempt + 1)); sleep 1 + done + [ "$attempt" -lt 30 ] || fail "restart command did not reach running state" + # This preserves the server state volume while replacing the actual server + # process behind nginx. A later TERM terminal result can only arrive after + # the Windows daemon has established a fresh reconciled WSS session. + compose restart server + attempt=0 + while [ "$attempt" -lt 45 ]; do + if rvc stat "$client_id" >/dev/null 2>&1; then break; fi + attempt=$((attempt + 1)); sleep 1 + done + [ "$attempt" -lt 45 ] || fail "server control socket did not recover after restart" + wait_client + rvc kill TERM "$client_id" "$issue" >/dev/null || fail "TERM after server restart failed" + attempt=0 + while [ "$attempt" -lt 45 ]; do + result=$(rvc stat "$client_id" "$issue" 2>/dev/null || true) + if printf '%s\n' "$result" | grep -q 'lifecycle=COMMAND_TERMINATED'; then + printf '%s\n' "$result" >"$fixture_dir/server-restart-reconnect.stat" + printf 'passed server-restart-reconnect issue=%s\n' "$issue" + return 0 + fi + case $result in *'lifecycle=COMMAND_FAILED'*|*'lifecycle=COMMAND_REJECTED'*|*'lifecycle=COMMAND_SUCCEEDED'*) fail "restart TERM command reached wrong terminal state: $result" ;; esac + attempt=$((attempt + 1)); sleep 1 + done + fail "restart TERM command did not reach terminal state" +} + collect() { if [ "$vm_prepared" = yes ]; then "$repo_root/scripts/windows/test-host" collect --run-id "$run_id" || true @@ -216,6 +262,7 @@ collect() { clean() { compose down --volumes --remove-orphans || true + remove_project_networks # A reset is the isolation boundary for the next run. Do not conceal a # failed shutdown/snapshot restore behind a successful-looking `clean`: # callers must repair or explicitly inspect the retained VM lease first. @@ -251,6 +298,7 @@ case $action in wait_client assert_stdin_close assert_signal_term + assert_server_restart_reconnect assert_context active-user no active-user assert_context active-user-elevated yes active-user-elevated "$repo_root/scripts/windows/test-host" run --run-id "$run_id" --fail-contexts ACTIVE_USER_ELEVATED diff --git a/scripts/windows/test-host b/scripts/windows/test-host index 303d9e0..3ae1095 100755 --- a/scripts/windows/test-host +++ b/scripts/windows/test-host @@ -23,6 +23,7 @@ Actions: logoff log off the sole active fixture user; use only after service installation collect copy bounded guest artifacts to the local test-run directory inspect read-only RVBox SCM state and bounded client log from a prepared run + logs read-only service-startup and client logs from a prepared run stop stop RVBox through SCM and request a graceful guest shutdown reset stop the guest if necessary, restore the declared baseline, and leave it off recover read-only fixture/run-state check for a stopped-resumable run @@ -87,7 +88,7 @@ while [ "$#" -gt 0 ]; do esac done -case $action in status|prepare|stage|install|run|logoff|collect|inspect|stop|reset|recover) ;; *) usage >&2; fail "unknown action $action" ;; esac +case $action in status|prepare|stage|install|run|logoff|collect|inspect|logs|stop|reset|recover) ;; *) usage >&2; fail "unknown action $action" ;; esac if [ "$action" != status ]; then [ -n "$run_id" ] || fail "$action requires --run-id" safe_id "$run_id" @@ -169,7 +170,7 @@ remote() { # retry. Lifecycle transitions remain single-attempt: their caller must # inspect/recover rather than risk a duplicate reset, shutdown, or logoff. case $remote_action in - status|recover|prepare-stage|stage|stage-create-root|stage-copy-exe|stage-copy-config|stage-copy-ca|collect|inspect|install|run) + status|recover|prepare-stage|stage|stage-create-root|stage-copy-exe|stage-copy-config|stage-copy-ca|collect|inspect|logs|install|run) retry_limit=4 ;; esac @@ -488,9 +489,9 @@ case "$action" in require_lease install -d -m 700 "$host_stage/artifacts" if [ "$(state)" = running ]; then - guest_run --exe 'C:\Windows\System32\cmd.exe' --wait-stdout --wait-stderr --unquoted-args -- \ - /d /s /c "sc.exe queryex RVBoxClient > \"$guest_root\\service-status.txt\" 2>&1 & echo RVBOX_GUEST_OK" >/dev/null || true - VBoxManage guestcontrol "$vm" --username "$guest_user" --passwordfile "$password_file" \ + provisioner_run --exe 'C:\Windows\System32\cmd.exe' --wait-stdout --wait-stderr --unquoted-args -- \ + /d /s /c "sc.exe queryex RVBoxClient > \"$guest_root\\service-status.txt\" 2>&1 & if exist \"C:/ProgramData/RVBox/service-startup.log\" copy /y \"C:/ProgramData/RVBox/service-startup.log\" \"$guest_root\\service-startup.log\" >NUL & if exist \"C:/ProgramData/RVBox/test-logs/rvbox.log\" copy /y \"C:/ProgramData/RVBox/test-logs/rvbox.log\" \"$guest_root\\rvbox.log\" >NUL & echo RVBOX_GUEST_OK" >/dev/null || true + VBoxManage guestcontrol "$vm" --username "$provisioner_user" --passwordfile "$provisioner_password_file" \ copyfrom "$guest_root" "$host_stage/artifacts" --recursive /dev/null 2>&1 || true fi printf 'collected host_stage=%s/artifacts\n' "$host_stage" @@ -500,7 +501,14 @@ case "$action" in require_lease [ "$(state)" = running ] || fail "inspect requires a running prepared VM" guest_run --exe 'C:\Windows\System32\cmd.exe' --wait-stdout --wait-stderr --unquoted-args -- \ - /d /s /c "sc.exe queryex RVBoxClient & sc.exe qc RVBoxClient & reg.exe query \"HKLM\\SYSTEM\\CurrentControlSet\\Services\\RVBoxClient\" /v ImagePath & reg.exe query \"HKLM\\SYSTEM\\CurrentControlSet\\Services\\RVBoxClient\" /v ObjectName & dir \"C:/ProgramData/RVBox\" & icacls \"C:/ProgramData/RVBox\" & certutil -hashfile \"$guest_root\\rvbox.exe\" SHA256 & \"$guest_root\\rvbox.exe\" --check-config --config \"$guest_root\\client.toml\" > \"$guest_root\\check-config.txt\" 2>&1 & type \"$guest_root\\check-config.txt\" & \"$guest_root\\rvbox.exe\" --service --config \"$guest_root\\client.toml\" > \"$guest_root\\direct-service-probe.txt\" 2>&1 & type \"$guest_root\\direct-service-probe.txt\" & if exist \"C:/ProgramData/RVBox/service-startup.log\" type \"C:/ProgramData/RVBox/service-startup.log\" & wevtutil qe System /q:\"*[System[(EventID=7000 or EventID=7009 or EventID=7031 or EventID=7034)]]\" /c:3 /rd:true /f:text & if exist \"C:/ProgramData/RVBox/test-logs/rvbox.log\" type \"C:/ProgramData/RVBox/test-logs/rvbox.log\" & echo RVBOX_GUEST_OK" + /d /s /c "sc.exe queryex RVBoxClient & sc.exe qc RVBoxClient & reg.exe query \"HKLM\\SYSTEM\\CurrentControlSet\\Services\\RVBoxClient\" /v ImagePath & reg.exe query \"HKLM\\SYSTEM\\CurrentControlSet\\Services\\RVBoxClient\" /v ObjectName & dir \"C:/ProgramData/RVBox\" & icacls \"C:/ProgramData/RVBox\" & certutil -hashfile \"$guest_root\\rvbox.exe\" SHA256 & \"$guest_root\\rvbox.exe\" --check-config --config \"$guest_root\\client.toml\" > \"$guest_root\\check-config.txt\" 2>&1 & type \"$guest_root\\check-config.txt\" & if exist \"C:/ProgramData/RVBox/service-startup.log\" type \"C:/ProgramData/RVBox/service-startup.log\" & wevtutil qe System /q:\"*[System[(EventID=7000 or EventID=7009 or EventID=7031 or EventID=7034)]]\" /c:3 /rd:true /f:text & if exist \"C:/ProgramData/RVBox/test-logs/rvbox.log\" type \"C:/ProgramData/RVBox/test-logs/rvbox.log\" & echo RVBOX_GUEST_OK" + ;; + logs) + assert_identity + require_lease + [ "$(state)" = running ] || fail "logs requires a running prepared VM" + provisioner_run --exe 'C:\Windows\System32\cmd.exe' --wait-stdout --wait-stderr --unquoted-args -- \ + /d /s /c "dir \"C:\\ProgramData\\RVBox\" & dir \"C:\\ProgramData\\RVBox\\test-logs\" & type \"C:\\ProgramData\\RVBox\\service-startup.log\" & type \"C:\\ProgramData\\RVBox\\test-logs\\rvbox.log\" & echo RVBOX_GUEST_OK" ;; stop) assert_identity diff --git a/test/linux-server/compose.yaml b/test/linux-server/compose.yaml index 0ede422..b193f86 100644 --- a/test/linux-server/compose.yaml +++ b/test/linux-server/compose.yaml @@ -32,6 +32,7 @@ services: - "${RVBOX_NATIVE_RUNTIME_DIR}/pki:/pki" - ./certgen.sh:/fixture/certgen.sh:ro entrypoint: ["/bin/sh", "/fixture/certgen.sh"] + networks: [native] networks: native: labels: { rvbox.native.run_id: "${RVBOX_NATIVE_RUN_ID}" } diff --git a/test/windowsnative/native_fixture_test.go b/test/windowsnative/native_fixture_test.go index 1b9b75f..b48edab 100644 --- a/test/windowsnative/native_fixture_test.go +++ b/test/windowsnative/native_fixture_test.go @@ -31,6 +31,8 @@ func TestNativeFixtureAssets_HP_HARNESS_20(t *testing.T) { "assert_stdin_close", "assert_signal_term", "rvc kill TERM \"$client_id\" \"$issue\"", + "assert_server_restart_reconnect", + "compose restart server", "rvc append \"$client_id\" \"$issue\" RVBOX_NATIVE_INPUT", "rvc close-stdin \"$client_id\" \"$issue\"", "clean --purge --yes", @@ -42,13 +44,13 @@ func TestNativeFixtureAssets_HP_HARNESS_20(t *testing.T) { } } testHost := read("scripts/windows/test-host") - for _, required := range []string{"xz -T0 -3", "accelerated_stage", "copy_stage_file", "--proxy", "--anyauth", "--continue-at", "rvbox.exe.xz", "retry_limit=4", "ConnectTimeout=10", "bundle transfer did not reach the expected SHA-256 manifest", "verified transfer_sha256", "reset-force-poweroff", "$run_root/.accelerated-stage.XXXXXX"} { + for _, required := range []string{"xz -T0 -3", "accelerated_stage", "copy_stage_file", "--proxy", "--anyauth", "--continue-at", "rvbox.exe.xz", "retry_limit=4", "ConnectTimeout=10", "bundle transfer did not reach the expected SHA-256 manifest", "verified transfer_sha256", "reset-force-poweroff", "$run_root/.accelerated-stage.XXXXXX", "service-startup.log", "test-logs/rvbox.log"} { if !strings.Contains(testHost, required) { t.Fatalf("native test-host is missing compressed transfer contract %q", required) } } compose := read("test/linux-server/compose.yaml") - for _, required := range []string{"../../bin/rvbox-server", "nginx:1.27-alpine", "rvbox.native.run_id", "RVBOX_NATIVE_RUNTIME_DIR"} { + for _, required := range []string{"../../bin/rvbox-server", "nginx:1.27-alpine", "rvbox.native.run_id", "RVBOX_NATIVE_RUNTIME_DIR", "certgen", "networks: [native]"} { if !strings.Contains(compose, required) { t.Fatalf("native Compose fixture is missing %q", required) } @@ -58,6 +60,16 @@ func TestNativeFixtureAssets_HP_HARNESS_20(t *testing.T) { if !strings.Contains(defaults, "//go:build !rvbox_native_test") || !strings.Contains(fixture, "//go:build rvbox_native_test") { t.Fatal("fixture-only context faults are not separated from release builds") } + service := read("cmd/rvbox/service_windows.go") + if !strings.Contains(service, "applyServiceDiagnosticsACL") || !strings.Contains(service, "D:P(A;;FA;;;SY)(A;;FA;;;BA)") { + t.Fatal("Windows service diagnostics are not readable by administrators") + } + testing := read("docs/testing.md") + for _, required := range []string{"server-process\nrestart", "new reconciled WSS session", "`logs` is the narrow read-only"} { + if !strings.Contains(testing, required) { + t.Fatalf("native workflow documentation is missing %q", required) + } + } } // TestProductionComposeAssets_HP_OPS_01 guards the deployment properties that