From 4b3bab9ffa5ecc9e5f2f1891464dc18dcc02cd7c Mon Sep 17 00:00:00 2001 From: Teodor Calin Date: Fri, 4 Sep 2026 10:53:18 +0300 Subject: [PATCH] fix: make auto-close deadlines honest --- cmd/shell/autoclose.go | 24 +++++++++++++++++++++ cmd/shell/autoclose_test.go | 42 +++++++++++++++++++++++++++++++++++++ cmd/shell/main.go | 36 +++++++++++++++++++------------ 3 files changed, 89 insertions(+), 13 deletions(-) diff --git a/cmd/shell/autoclose.go b/cmd/shell/autoclose.go index 069e307..9a58207 100644 --- a/cmd/shell/autoclose.go +++ b/cmd/shell/autoclose.go @@ -117,6 +117,30 @@ func parseCloseDeadline(value string, now time.Time) (time.Time, error) { return time.Time{}, fmt.Errorf("invalid auto-close value %q (try 5m, 2h, 3d, 1w, 2mo, or an ISO date)", value) } +// processCloseDeadline starts relative durations when the shared process is +// ready to launch. Network/session setup must not consume a duration such as +// --auto-close 5s. Absolute dates retain their original wall-clock meaning. +func processCloseDeadline(value string, parsedDeadline, processStart time.Time) time.Time { + value = strings.TrimSpace(value) + if strings.HasPrefix(strings.ToLower(value), "in ") { + value = strings.TrimSpace(value[3:]) + } + if deadline, ok := parseRelativeDeadline(value, processStart); ok { + return deadline + } + return parsedDeadline +} + +// boundedCloseDeadline prevents the local process from outliving a deadline +// the relay has already declared. A zero requested deadline still means the +// task itself controls its lifetime. +func boundedCloseDeadline(requested, sessionExpiry time.Time) time.Time { + if requested.IsZero() || sessionExpiry.IsZero() || !sessionExpiry.Before(requested) { + return requested + } + return sessionExpiry +} + func parseRelativeDeadline(value string, now time.Time) (time.Time, bool) { remainder := strings.ReplaceAll(strings.TrimSpace(value), " ", "") if remainder == "" { diff --git a/cmd/shell/autoclose_test.go b/cmd/shell/autoclose_test.go index e47fc4c..ed40b23 100644 --- a/cmd/shell/autoclose_test.go +++ b/cmd/shell/autoclose_test.go @@ -52,6 +52,48 @@ func TestParseCloseDeadlineRejectsInvalidValues(t *testing.T) { } } +func TestProcessCloseDeadlineStartsRelativeDurationAfterSetup(t *testing.T) { + parsedAt := time.Date(2026, time.August, 19, 23, 40, 0, 0, time.UTC) + processStart := parsedAt.Add(12 * time.Second) + parsedDeadline, err := parseCloseDeadline("25ms", parsedAt) + if err != nil { + t.Fatal(err) + } + + got := processCloseDeadline("25ms", parsedDeadline, processStart) + want := processStart.Add(25 * time.Millisecond) + if !got.Equal(want) { + t.Fatalf("processCloseDeadline() = %v, want %v", got, want) + } +} + +func TestProcessCloseDeadlinePreservesAbsoluteTime(t *testing.T) { + parsedAt := time.Date(2026, time.August, 19, 23, 40, 0, 0, time.UTC) + processStart := parsedAt.Add(12 * time.Second) + parsedDeadline := parsedAt.Add(time.Hour) + + got := processCloseDeadline("2026-08-20T00:40:00Z", parsedDeadline, processStart) + if !got.Equal(parsedDeadline) { + t.Fatalf("processCloseDeadline() = %v, want %v", got, parsedDeadline) + } +} + +func TestBoundedCloseDeadlineUsesRelayExpiry(t *testing.T) { + now := time.Date(2026, time.August, 19, 23, 40, 0, 0, time.UTC) + sessionExpiry := now.Add(12 * time.Hour) + requested := now.Add(30 * 24 * time.Hour) + + if got := boundedCloseDeadline(requested, sessionExpiry); !got.Equal(sessionExpiry) { + t.Fatalf("boundedCloseDeadline() = %v, want %v", got, sessionExpiry) + } + if got := boundedCloseDeadline(now.Add(time.Hour), sessionExpiry); !got.Equal(now.Add(time.Hour)) { + t.Fatalf("boundedCloseDeadline() shortened a valid deadline to %v", got) + } + if got := boundedCloseDeadline(time.Time{}, sessionExpiry); !got.IsZero() { + t.Fatalf("boundedCloseDeadline() changed task-bound lifetime to %v", got) + } +} + func TestNormalizeAutoCloseArguments(t *testing.T) { now := time.Date(2026, time.August, 19, 23, 40, 0, 0, time.UTC) tests := []struct { diff --git a/cmd/shell/main.go b/cmd/shell/main.go index 833b776..639e7d8 100644 --- a/cmd/shell/main.go +++ b/cmd/shell/main.go @@ -66,7 +66,7 @@ func run(arguments []string, stdout, stderr io.Writer) int { fmt.Fprintln(stderr, "shell: --e2ee and --no-e2ee cannot be used together") return 2 } - closeDeadline, err := parseCloseDeadline(autoClose.value, now) + parsedCloseDeadline, err := parseCloseDeadline(autoClose.value, now) if err != nil { fmt.Fprintf(stderr, "shell: %v\n", err) return 2 @@ -124,20 +124,11 @@ func run(arguments []string, stdout, stderr io.Writer) int { signalContext, stopSignals := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM, syscall.SIGHUP) defer stopSignals() - var processContext context.Context - var cancelProcess context.CancelFunc - if closeDeadline.IsZero() { - processContext, cancelProcess = context.WithCancel(signalContext) - } else { - processContext, cancelProcess = context.WithDeadline(signalContext, closeDeadline) - } - defer cancelProcess() - client := api.NewClient(strings.TrimRight(*server, "/"), "shell/"+version) var session api.Session if *persistentState != "" { session, password, err = preparePersistentSession( - processContext, client, *persistentState, filepath.Base(command[0]), *readOnly, true, password, + signalContext, client, *persistentState, filepath.Base(command[0]), *readOnly, true, password, ) } else { if encrypted && password == "" { @@ -150,7 +141,7 @@ func run(arguments []string, stdout, stderr io.Writer) int { frameCipher, encryptionFragment, err = e2ee.Generate(password) } if err == nil { - session, err = client.CreateSession(processContext, filepath.Base(command[0]), *readOnly, encrypted, false) + session, err = client.CreateSession(signalContext, filepath.Base(command[0]), *readOnly, encrypted, false) } session.Cipher = frameCipher session.ShareURL += encryptionFragment @@ -162,6 +153,25 @@ func run(arguments []string, stdout, stderr io.Writer) int { return 1 } + processStartedAt := time.Now() + closeDeadline := processCloseDeadline(autoClose.value, parsedCloseDeadline, processStartedAt) + closeDeadline = boundedCloseDeadline(closeDeadline, session.ExpiresAt) + if !closeDeadline.IsZero() && !closeDeadline.After(processStartedAt) { + err = fmt.Errorf("auto-close deadline elapsed before the process could start") + sendBackgroundResult(backgroundLaunchResult{OK: false, Error: err.Error()}) + fmt.Fprintf(stderr, "shell: %v\n", err) + return 1 + } + + var processContext context.Context + var cancelProcess context.CancelFunc + if closeDeadline.IsZero() { + processContext, cancelProcess = context.WithCancel(signalContext) + } else { + processContext, cancelProcess = context.WithDeadline(signalContext, closeDeadline) + } + defer cancelProcess() + var closesAt *time.Time if !closeDeadline.IsZero() { deadline := closeDeadline @@ -176,7 +186,7 @@ func run(arguments []string, stdout, stderr io.Writer) int { Persistent: session.Persistent, Command: displayCommand(launch.DisplayArguments), PID: os.Getpid(), - StartedAt: now, + StartedAt: processStartedAt, ClosesAt: closesAt, }) if controlError != nil {