From 9cfba6947575877e497ca6e11df4780f12aa6e3c Mon Sep 17 00:00:00 2001 From: Ari Mayer Date: Wed, 1 Jul 2026 17:26:34 -0400 Subject: [PATCH] fix(sync): make the rclone probe timeout configurable (#137) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-unlock metadata probe (rclone lsjson) was bounded at a hardcoded 8s. A slow-but-alive remote on a high-latency backend would exceed that, be stamped as a failure, and enter the #133 failure-backoff — deferring pulls/pushes for the whole window even though the remote was reachable. Add sync.probe_timeout_seconds, mirroring the pull_ttl_seconds tri-state via a resolveProbeTimeout helper: - 0 -> built-in default (8s) - >0 -> that many seconds - <0 -> disable the bound (unbounded probe) Wired into the execExecutor built in NewService. Only the metadata probe (Run) is bounded; the heavy pull/push transfers (RunNoOutput) stay unbounded as before. Default is unchanged at 8s. Also documents both probe_timeout_seconds and the previously-undocumented pull_ttl_seconds in the configuration reference. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014kVLjbUL4F4CkoA7RbMYy8 --- CHANGELOG.md | 1 + docs/03-reference/configuration.md | 2 ++ internal/config/config.go | 8 ++++++ internal/sync/sync.go | 18 ++++++++++++- internal/sync/sync_test.go | 43 ++++++++++++++++++++++++++++++ 5 files changed, 71 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 204b6c74..74a2210a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Added +- **Sync: configurable remote-probe timeout** (#137) — the pre-unlock metadata probe (`rclone lsjson`) was bounded at a hardcoded 8s, so a slow-but-alive remote on a high-latency backend would time out, be misclassified as failed, and enter the failure-backoff (#133) — deferring pulls/pushes for the whole window even though the remote was reachable. New `sync.probe_timeout_seconds` mirrors the `pull_ttl_seconds` tri-state: `0` uses the built-in default (8s), a positive value raises (or lowers) the bound, and a negative value disables it (unbounded probe). Only the metadata probe is bounded — the heavy pull/push transfers remain unbounded regardless. - **Background agent (`pass-cli agent`)** (#116) — an optional daemon that unlocks the vault once and holds it in memory, answering read-only credential lookups over a local unix socket so `exec`/`export`/`inject` need no master-password prompt and no key derivation on each call. It serves resolved field **values only** — the master password and derived key never cross the socket. Auto-locks after `--idle` inactivity (default 15m) and always after `--max-ttl` (default 8h), and locks + exits on SIGINT/SIGTERM. **`pass-cli agent start`** backgrounds the agent (unlock once on your terminal, then detach — no shell `&` needed); **`pass-cli agent serve`** (or bare `pass-cli agent`) runs it in the foreground. **`agent stop`** zeroes the resident secrets and stops the agent (freeing the socket so the next command falls back to direct-open; re-run `agent start` to re-establish it); `agent status` reports its state (never prints secrets). When no agent is running, every command **transparently falls back** to opening and unlocking the vault directly, so the agent is a pure optimization, never a dependency. Socket path: `$PASS_CLI_AGENT_SOCK`, else `$XDG_RUNTIME_DIR/pass-cli/agent.sock`, else `~/.pass-cli/agent.sock` (directory `0700`, socket `0600`). Connections are additionally authorized by peer credential — only a process owned by the same user may talk to the agent, and any failure to read the credential is a rejection (fail-closed): Linux via `SO_PEERCRED`, macOS via `getsockopt(LOCAL_PEERCRED)`. (Windows would use a named pipe + ACL; the Windows agent is not yet implemented and falls back to direct-open.) POSIX only for now; a Windows named-pipe transport is planned. - **`export` command — print shell statements that set credentials as env vars** (#115) — `pass-cli export` emits `export NAME='value'` for `eval`/`source`, the blessed replacement for `VAR="$(pass-cli get …)"`: `eval "$(pass-cli export --set GITHUB_TOKEN=github)"`. Uses the same mapping grammar as `exec` (repeatable `--set ENV_NAME=service[/field]` and the convenience form `pass-cli export `, which derives the name from the service), and the same `-f/--field` selection. `--format sh|fish|powershell` selects the shell syntax (default `sh`). Read-only like `exec` (records no usage, triggers no sync push). Because `export` output is meant to be eval'd, env names are validated against `[A-Za-z_][A-Za-z0-9_]*` before the vault is opened, and every field value is shell-quoted per target shell. - **`inject` command — render a template, substituting `${pass:service/field}` references** (#115) — `pass-cli inject` reads a template from `--in-file`/stdin and writes it back with every `${pass:service/field}` reference replaced by the credential's value: `echo 'postgres://app:${pass:db/password}@host/db' | pass-cli inject`. This is the composite/derived-secret tool (whole config files, connection strings) that a single `ENV=service` mapping cannot express. Only `${pass:...}` is special (`$VAR`/`${VAR}`/`$(...)` pass through); resolution is single-pass and fail-closed (an unknown or malformed reference errors and writes nothing). `--out-file` is created `0600`. Read-only. diff --git a/docs/03-reference/configuration.md b/docs/03-reference/configuration.md index d9952819..2d1affd9 100644 --- a/docs/03-reference/configuration.md +++ b/docs/03-reference/configuration.md @@ -134,6 +134,8 @@ sync: |--------|------|---------|-------------| | `enabled` | bool | `false` | Enable/disable rclone sync | | `remote` | string | `""` | rclone remote and path | +| `pull_ttl_seconds` | int | `0` (30s) | Window in which a command serves the local vault without re-probing the remote; also the failure-backoff window. `0` uses the default (30s); negative disables the gate (probe every command). | +| `probe_timeout_seconds` | int | `0` (8s) | Timeout for the pre-unlock remote metadata probe. Raise it for a slow/high-latency remote so it isn't misclassified as failed. `0` uses the default (8s); negative disables the bound (unbounded probe). Heavy pull/push transfers are always unbounded. | **Sync Behavior**: - **Pull**: Happens once per CLI session (before first vault access) diff --git a/internal/config/config.go b/internal/config/config.go index bca86902..f1b32a6c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -46,6 +46,14 @@ type SyncConfig struct { // remote) the next commands skip the probe until it expires, so a dead remote // costs the probe timeout at most once per window instead of on every call. PullTTLSeconds int `mapstructure:"pull_ttl_seconds"` + // ProbeTimeoutSeconds bounds the pre-unlock remote metadata probe (rclone + // lsjson). A slow-but-alive remote whose probe exceeds this bound is treated + // as failed and enters the failure-backoff, so users on a high-latency + // backend can raise it to avoid being misclassified as down. 0 uses the + // built-in default (8s); a negative value disables the bound (unbounded + // probe). Only the metadata probe is bounded — the heavy pull/push transfers + // are always unbounded. Mirrors the pull_ttl_seconds tri-state. + ProbeTimeoutSeconds int `mapstructure:"probe_timeout_seconds"` } // ValidationResult represents the outcome of checking configuration correctness diff --git a/internal/sync/sync.go b/internal/sync/sync.go index fbf372bd..ee6d8df6 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -188,11 +188,27 @@ func NewService(cfg config.SyncConfig) *Service { } return &Service{ config: cfg, - executor: &execExecutor{runTimeout: defaultProbeTimeout}, + executor: &execExecutor{runTimeout: resolveProbeTimeout(cfg.ProbeTimeoutSeconds)}, pullTTL: ttl, } } +// resolveProbeTimeout maps the configured probe_timeout_seconds to a duration: +// 0 uses the built-in default; a positive value sets the bound; a negative +// value disables the bound (unbounded probe). Mirrors the pull_ttl_seconds +// tri-state. Only the metadata probe (Run) is affected — the heavy transfers +// stay unbounded regardless. +func resolveProbeTimeout(seconds int) time.Duration { + switch { + case seconds > 0: + return time.Duration(seconds) * time.Second + case seconds < 0: + return 0 // disabled: unbounded probe + default: + return defaultProbeTimeout + } +} + // NewServiceWithExecutor creates a new sync service with a custom command executor (for testing). func NewServiceWithExecutor(cfg config.SyncConfig, executor CommandExecutor) *Service { return &Service{ diff --git a/internal/sync/sync_test.go b/internal/sync/sync_test.go index 03c907e7..7c128f2e 100644 --- a/internal/sync/sync_test.go +++ b/internal/sync/sync_test.go @@ -936,3 +936,46 @@ func TestSmartPush_DefersWhenFailureBackoffActive(t *testing.T) { t.Errorf("expected LastPushHash untouched on defer, got %q", st.LastPushHash) } } + +func TestResolveProbeTimeout(t *testing.T) { + tests := []struct { + name string + seconds int + want time.Duration + }{ + {"zero uses default", 0, defaultProbeTimeout}, + {"positive sets bound", 5, 5 * time.Second}, + {"negative disables bound", -1, 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := resolveProbeTimeout(tt.seconds); got != tt.want { + t.Errorf("resolveProbeTimeout(%d) = %s, want %s", tt.seconds, got, tt.want) + } + }) + } +} + +func TestNewServiceWiresProbeTimeout(t *testing.T) { + tests := []struct { + name string + seconds int + want time.Duration + }{ + {"default", 0, defaultProbeTimeout}, + {"custom", 12, 12 * time.Second}, + {"disabled", -1, 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc := NewService(config.SyncConfig{ProbeTimeoutSeconds: tt.seconds}) + ex, ok := svc.executor.(*execExecutor) + if !ok { + t.Fatalf("executor is %T, want *execExecutor", svc.executor) + } + if ex.runTimeout != tt.want { + t.Errorf("runTimeout = %s, want %s", ex.runTimeout, tt.want) + } + }) + } +}