Main sync: C07 DR bundles, C08 transport/secrets - #20
Merged
Merged
Conversation
…ure-text parsing (C08-1)
Executor.Run's (trimmedStdout, error) contract folds the exit code, the
real stderr, and the ran-but-failed vs never-completed distinction into
one error string, so call sites parsed semantics out of generic failure
text ("not found", "status 127") — which breaks when a tool changes
wording, localizes, or a wrapper message contains the phrase for
another reason.
Add internal/ssh.Result: Stdout/Stderr as bounded raw bytes (1 MiB per
stream, Truncated flag), ExitCode (0..255, -1 when no status arrived),
TimedOut and Canceled as distinct context-outcome flags, and Err set
only when the command did not complete (transport, cancel, timeout) —
a clean non-zero exit is a result, not a transport failure.
RunDetailed/RunInputDetailed are package-level helpers over any
Executor: native capture for RemoteExecutor (separate channel streams,
exit status from the SSH channel request), LocalExecutor (separate
pipes, process exit status, existing process-group kill semantics), and
MockExecutor (registered errors carrying "exit status N" derive the
code; stdin payloads now recorded in MockExecutor.Inputs for the
secret-transport pins). A generic fallback covers other Executor
implementations with an honest degraded shape (exit code -1 unless
derivable). Executor interface unchanged — purely additive.
RunInputDetailed exists because RunInput's contract discards
stdout/stderr entirely, hiding the diagnostics a failing stdin-fed
pipeline needs (docker login refusals, su authentication failures) —
the next commits route secret transport through it.
Call sites fixed to consume the structured fields:
- doctor compatibility: absence of the optional server-side teploy
binary is now judged by exit code 127, not by matching "not found" /
"no such file" / "status 127" substrings in the wrapped error. A
non-127 failure whose text merely contains "not found" (e.g. an
ld.so error) now warns instead of reporting a bogus absence — pinned
in tests.
- doctor registry: classifyRegistryFailure reads the tool's own
stderr from the structured result rather than the folded error text
(docker's CLI exits 1 for every failure class, so the exit code
cannot classify; stderr is the authoritative signal, teploy's own
wrapper words no longer participate in the classification).
- registry list: the old `cat config.json 2>/dev/null || echo '{}'`
turned an unreadable config into "No registries configured" — a read
failure becoming empty state. The framed read distinguishes
confirmed file absence (legitimate empty) from a read failure
(error); pinned in tests.
Existing mock-based doctor tests pass unchanged where their registered
errors carry exit statuses ("exit status 127: ...") — the derivation
rule is documented as the mock contract.
Gates: go build ./... && go vet ./... clean; go test ./internal/ssh
./internal/cli -count=1 ok; -race on internal/ssh ok; gofmt clean on
touched files (pre-existing strays in internal/cli untouched).
…e (C08-2) A hung remote command previously hung the CLI forever whenever the caller's context carried no deadline (most do): RunStream/RunInput only return on the command's completion or ctx cancellation, so a server that accepts the session and never sends an exit status owned the process. The handshake was bounded (dialWithContext, 15s) but nothing bounded the commands that ran over the connection it established. Add ConnectConfig.CommandTimeout: when > 0, every command on that connection — Run, RunStream, RunInput, RunDetailed — runs under a context with at least that deadline; a caller-supplied earlier deadline always wins, 0 keeps the caller-controlled behavior (deploys, log tails, and other long-running callers are unchanged). Expiry surfaces through the structured Result as TimedOut (C08-1), and the session is torn down exactly as an explicit cancel already was (SIGTERM, Close, wait for the session goroutine). Pinned with a new in-process SSH server fixture (internal/ssh/ sshtest_test.go): a real x/crypto/ssh server on 127.0.0.1 exercising Connect's full path — dial, handshake, TOFU host-key enrollment with a fresh $HOME, session multiplexing. Pins: - structured status over the real wire: the exit code arrives from the server's exit-status request (42 decoded), stdout/stderr stay separated, non-zero exit leaves Err nil; - bounded lifetime: a server that never answers dies at the 500ms CommandTimeout with TimedOut set, and the connection remains usable for the next command afterward; - mid-run cancellation returns promptly with Canceled set (and TimedOut clear — the two outcomes stay distinguishable). Local process-group cleanup (A28) is already pinned for LocalExecutor; TestRunDetailed_LocalExecutor_CancellationKillsGroup extends it to the structured path: a descendant that would write a marker after cancellation never writes it. Gates: go build ./... && go vet ./... clean; go test ./internal/ssh -count=1 -race ok; gofmt clean on touched files.
… slice 1) Define the schema-versioned disaster-recovery bundle (C07): a manifest carrying app state (verbatim state.json), release records, the applied app manifest, secret REFERENCES by default (encrypted material only on explicit --include-secrets, age key only on --include-age-key), routing/TLS references, and per-snapshot consistency records. Data-only volume backups stay clearly named as such — an app with no on-server state is refused with the pointer to 'teploy backup create'. Engine awareness is built on the EXISTING backup system, not a fork: - AccessoryBackup's inline per-engine switch is extracted into planAccessoryDump (classification + method + consistency + exact dump command, byte-identical to the pinned originals), now shared by the bundle path. - Restore-command templates are extracted into shared builders (postgres/mysql/mongo/redis + redisAOFPreflight), parameterized by container name so scratch validation containers can reuse them. - extractToStagingThenPromote's two-phase promotion core is factored into promoteStaged (same recovery discipline: originals moved aside before copy, rollback restores them, recovery-incomplete keeps artifacts named) for reuse by the DR cutover path. Consistency honesty (a tar of a live database volume is not a consistent backup): engine dumps record engine-consistent, redis records engine-snapshot (BGSAVE-acknowledged), generic tars record crash-consistent with an explicit note; app volumes default to crash-consistent with the caveat in the manifest, upgrade to quiesced via --stop-app (stop->tar->restart, order pinned by test) or an operator-asserted --quiesced-volume. Volume-path engine detection is a labeled heuristic: /data is matched EXACTLY only (substring matching misclassified /app/data as redis — caught by the new tests), and every pattern match still records crash-consistency with a 'NOT a <engine>-consistent backup' note. Bundles store through a BundleStore interface: S3 (aws CLI, manifest uploaded LAST as the completeness marker) or a plain server directory (offline bundles). Non-secret engine params (db/user names) and the accessory image travel in SnapshotRecord so an isolated restore can boot scratch engines without credentials. Hermetic tests: manifest contract + upload ordering, no-state refusal, pattern-heuristic honesty (incl. the /app/data fix), stop-app quiescence ordering, include-secrets opt-in (default bundles nothing), recovery-plan documentation (accessory upgrade, credential rotation, storage-path keying, TLS material), manifest schema/kind refusal, store listing requires manifests. Gates: build/vet clean, full suite 26/26 packages ok.
… receipt (C07 slice 2) RestoreBundleIsolated lands a DR bundle in /var/tmp/teploy-dr staging (deterministic path, pattern-guarded wipe), validates it against THROWAWAY scratch containers, and writes a measured receipt. Live overwrite is never the default — nothing under /deployments is touched until the separate, explicit CutoverBundle runs. Ordering is the safety property (pinned by tests): 1. Read-only preflight: manifest fetch/parse (schema, kind, app match), secrets preflight — in references mode every referenced key must exist on the target, and the missing ones abort BEFORE any staging, download, or docker command runs. 2. Download + gzip -t integrity proof on every .gz member — a corrupt bundle dies before any engine boots. 3. Decrypt proof for encrypted bundles (bundled age key, else the target's) — failure aborts with staging-only footprint. 4. Extract volumes/generic tars into staging; boot scratch engines (postgres/mysql/mongo/redis validators reusing the shared restore builders + waitReady), restore dumps, run data checks (tables>0, dbs, keys); boot the recorded app image on the staged volumes for the application check. Scratch containers are always torn down. 5. Receipt with RPO (restore start minus bundle creation = the data-age window) and RTO (measured restore+validation wall time), per-check results, staging path, and the cutover command as next step. CutoverBundle is the mutation step: requires a staged restore whose receipt passed validation (ErrValidationRequired otherwise), re-runs the secrets preflight, cross-checks bundle accessories/volumes against the restore-time teploy.yml (missing definitions abort before anything is stopped), then under the app lock (AcquireLockFenced): stops live containers, restores engine dumps into fresh accessories (pre-existing engine dirs moved aside as named recovery copies — pg_dump without --clean must not land over populated tables), promotes generic/accessory trees and volumes through the two-phase promoteStaged, and installs state.json + release records + secret material LAST so a failed promotion never leaves the target claiming a generation it lacks. Recovery dirs are deliberately kept and named in the receipt; stopped containers are restarted on failure. Hermetic failure-injection tests pin the C07 acceptance cases: missing keys fail before any mutation (only read-only commands observed), corrupt bundle fails before engines, encrypted-without-decryptable-key fails before engines, injected copy failure mid-promotion executes the rollback and preserves originals, injected accessory start failure aborts before any promotion or state install and restarts stopped containers, refused cutover (unvalidated/failed receipt, config mismatch, preflight) never stops anything. Happy paths pin the receipt math (RPO 7200s / RTO 90s from injected clocks), scratch teardown, the engine dump landing in the live accessory, and release-record/state installation.
…tover Wire the C07 recovery surface into the CLI as its own 'teploy dr' family next to the existing data-only 'teploy backup' commands (the long help and error text keep the two clearly named apart). Bundle targets select --bucket (S3 via the existing s3Config creds/env handling) or --dir (server directory, offline bundles); choosing both is an error. create exposes --include-secrets/--include-age-key (--include-age-key requires --include-secrets), --stop-app and --quiesced-volume; restore prints the RPO/RTO receipt (JSON with --json) and exits non-zero when validation failed; cutover refuses to run without a validated staged restore. All commands connect through the standard connectForApp path.
…xposed paths (C08-3) Inventory of every path that still moved secret material through argv (or an on-disk artifact) where a stdin/private-file channel exists; each is fixed and pinned. Verified-clean paths listed at the bottom. FIXED — docker-exec paths: - openbao bao() (ALL vault operations): BAO_TOKEN was embedded in the `docker exec ... sh -c 'BAO_TOKEN=... bao ...'` string — that is the docker exec process's command line on the HOST, so the root token sat in the server's process list for every init/kv/approle/database/raft call. The inner shell now reads one stdin line into the env and execs bao; the token rides the session pipe. - openbao Put(): kv VALUES were shell-quoted onto the same argv. Now `kv put <path> -` with a JSON object on stdin (token line + payload on one pipe); values with any byte content (no quoting surface) never enter a command string. shellSingleQuote is gone with them. - openbao writeAppPolicy(): put the root token in the docker exec argv directly. Token + raw HCL now ride stdin; the base64 detour existed only to survive argv quoting and is gone. - openbao EnableDatabaseSecrets/EnableStaticRole(): the DB admin password was shell-quoted into the `write database/config/...` argv. Config and role writes now send JSON over stdin (`write <path> -`). - backup AccessoryBackup/AccessoryRestore (mysql/mariadb): the comment said "never a command-line flag" while `docker exec -e MYSQL_PWD=<pwd>` put the password on the DOCKER EXEC argv — host-visible for the life of the dump/restore. Now a 0600 env-file inside the (already 0700) workspace consumed by `docker exec --env-file`; the backup path removes it with the workspace on every exit path, and the restore path — which deliberately KEEPS its workspace on failure for inspection — removes the credential file by name first: keep the SQL, never the secret. Failure before staging aborts with no dump run. FIXED — other remote paths: - registry login: the password was embedded in the remote command string (`printf '%s' '<pw>' | docker login --password-stdin`) — visible in the session shell's argv and in command-bearing errors. docker already reads stdin; it is now fed over the session (RunInputDetailed), whose structured stderr also surfaces login refusals the discard-both RunInput contract used to hide. - setup su path: the root password was embedded in a script uploaded to /tmp (an on-disk artifact, best-effort removed). su now reads it from the session stdin (installSudoViaSu); no temp file exists at any point, and the Authentication-failure / TEPLOY_SUDO_OK semantics are preserved via the structured stdout+stderr. - setup VPN join: tailscale --authkey / netbird --setup-key literals sat in the detached join command line. The credential is now staged as a 0600 /tmp file; the detached shell reads it into the provider's documented env var (TS_AUTHKEY / NB_SETUP_KEY), removes the file BEFORE exec'ing the provider, then execs — on-disk window is upload-to-join, argv exposure is zero. An upload failure aborts before anything joins. VERIFIED CLEAN (no change): internal/secret age store (value over stdin, ciphertext via 0600 mktemp+mv), env set / kv set / template --var-stdin local contracts (cb7c0fc), openbao container seal env (0600 env-file + docker run --env-file), ShipAudit observe token (HTTP header, not argv), generic docker.Exec/ExecStream (transport only; the bao layer was the secret-bearing caller). Supporting: docker.Client.ExecInput (container-exec analogue of RunInput with captured stdout + structured failure text), and MockExecutor now records RunInput stdin payloads (Inputs) so every pin asserts BOTH halves: secret absent from every recorded command, secret present on the stdin pipe. Gates: go build ./... && go vet ./... clean; go test ./... -count=1 ok (cli, ssh, backup, openbao, docker, secret); -race on ssh/backup/ openbao ok; gofmt clean on touched files (pre-existing strays untouched).
…tion (C08-4) Two simultaneous first connects raced the old trust-on-first-use callback three ways: the knownhosts database was captured at Connect() time (so a concurrent enrollment was invisible and every racing process re-enrolled, duplicating lines), the append had no lock (concurrent O_APPEND writes could interleave), and an interrupted write could leave a torn line — which fails knownhosts parsing and locks EVERY future connection out until the file is hand-repaired. enrollHostKey (hostkey.go) replaces the callback body; the v0.1.37 diagnostics (mismatchHint) are kept verbatim as the identity-change path. Changes: - verify+enroll runs under an exclusive flock on <known_hosts>.teploy-lock (flock rather than locking known_hosts itself, so stock ssh(1) — which takes no locks — never interacts with ours; the lock file is never unlinked, closing the unlinked-inode race). Non-Unix builds get a documented best-effort O_EXCL fallback with a bounded stale-lock takeover — the resident mode targets Linux. - the known_hosts database is re-read FRESH under the lock, so the second process sees the first's enrollment and verifies instead of duplicating it. - enrollment is temp-file + fsync + rename: a crash mid-write leaves the previous complete file (never a torn line), the rename preserves every other process's entries, and an existing file's permissions are kept. - RENAME vs IDENTITY CHANGE: an unknown host presenting a key already trusted under a DIFFERENT hostname is the same machine re-addressed (VPN IP rotation, DNS change) — possession of the host key is what TOFU established — so the new name is enrolled with a stderr note. An unknown key for a KNOWN host remains an identity change and fails closed with the existing mismatch diagnostics. Revoked keys and malformed databases fail closed exactly as before. File format unchanged (plain knownhosts lines, appended), so older teploy builds and stock ssh(1) read what we write and vice versa. Pins: 16 concurrent first connects to one host all succeed with EXACTLY one line appended and a still-parseable file; concurrent first connects to DISTINCT hosts lose nothing (read-modify-write, not blind append); rename enrolls the new name while an identity change is refused without touching the file; and two full Connect()s run simultaneously against the in-process SSH server with one shared fresh $HOME — the end-to-end race — both succeeding with one enrollment. Gates: go build ./... && go vet ./... clean; go test ./internal/ssh -count=1 -race ok; gofmt clean on touched files.
…ing (C07 slice 4) Complete the in-flight iteration the C07 lane left uncommitted, driven by the real fixture rather than mocks: - AppRun derivation: the isolated restore's application check boots the deployed image WITH the container command from the newest release record (state.json has no cmd; without argv most images exit instantly and the check proves nothing). Carried in the manifest as app_run; element-wise quoted at the docker run sink. - Cutover pre-cutover preservation moves each VOLUME directory aside (env/credential files in the accessory dir stay for the fresh accessory) with a root-container fallback (the accessory's own image, --user 0) for engine-chowned data dirs a non-root deploy user cannot rename, and an existence guard so a configured volume key with nothing on disk (volume added post-bundle, re-run after a partial failure) skips instead of aborting; no empty recovery dirs recorded. - DirBundleStore.Upload creates the member SUBDIRECTORY before copy; the bundle workspace volumes/ dir exists before tar writes into it. - dr_integration_test.go (integration-tagged, C01/C03 fixture contract, skips cleanly when env unset): full round trip against real docker+postgres (create from live data, isolated restore with data+app checks and measured RPO/RTO, /deployments proven untouched, cutover lands the rows, replaces the drifted volume, preserves the pre-cutover copy, reinstalls state), corrupt-bundle refusal, injected mid-promotion copy failure preserving exactly the originals, missing references-mode key aborting before staging. 4/4 PASS against the colima fixture (docker 29.5.2). - gofmt alignment fixes in redis_restore_test.go / retention_test.go. AUDIT_OPEN.md: C07 slice entry + T40 marked resolved by it. Gates: go build ./... clean; go vet ./... clean; go vet -tags integration ./internal/backup clean; go test ./... -count=1 all 26 packages ok; integration battery 4/4 PASS live.
…y tail) The provider Join methods (teploy network <provider> join) still put the auth key / setup key literal in the detached join command line — the session shell's argv, visible in the server's process list for the life of the join. Same class as the setup join C08-3 fixed; missed because it lives in internal/network. The staged-file transport moves to the network package as StageJoinCredential (0600 file, generated path, upload failure aborts before anything joins) + EnvVarJoinShell (the consuming shell reads the file into the provider's documented env var, removes the file BEFORE exec'ing the provider, then execs). Both entry points share it: the three provider Joins (TS_AUTHKEY / NB_SETUP_KEY) and setup's joinVPNMesh, whose private copy is deleted. Headscale's login-server URL is not a secret and stays a plain flag. The three tests that pinned the old shape (--authkey=/--setup-key present, key visible) now pin its absence plus the env-var/file shape and the exactly-once 0600 upload. Gates: go build ./... && go vet ./... clean; go test ./internal/network ./internal/cli -count=1 ok; -race on network ok; gofmt clean.
…ery (C08-5) Provisioning becomes an ordered list of named setupStages, each check-then-act idempotent and each carrying the affected-resource description the preflight prints BEFORE anything runs; failures name the stage that died. The password-path authorized_keys install gains a grep guard so an interrupted-and-rerun setup cannot stack duplicate entries. internal/ssh.ReconnectingExecutor adds bounded connection recovery: on a TRANSPORT-class failure (no exit status, not a caller cancellation) it redials through the setup ConnectConfig — budget 3, backoff 500ms to 2s — and retries exactly the dead invocation. Commands that RAN and failed are never retried here. RunStream refuses to retry once any output reached the caller (a retry would duplicate it; the error names the byte count). RunInput and Upload buffer their payload so the redialed attempt resends it whole — the first attempt consumed the reader. A native runDetailed delegation (registered in runDetailedWithLimit's type switch) keeps RunInputDetailed's structured fields intact through the wrapper; the generic fallback would lose the stdout markers installSudoViaSu classifies. runSetup routes the whole flow — provisioning, hardening, network join, and the VPN reconnection — through the wrapper; both sudo paths and the hardening steps are documented idempotent, which is the wrapper's stated precondition. The previous session's fragment is completed rather than reverted: the stage refactor compiled only after restoring six lost trailing returns (the reindent had dropped them), the RunStream guard was dead code behind an unconditional retry, and RunInput/Upload retried with an exhausted reader. Pins: interruption at the network stage fails naming the stage; the re-run against the same partially-provisioned server completes skipping everything done (no installer script, no apt install, no docker run, no Caddyfile rewrite); a one-shot transport death mid-flow recovers and completes with the dead command retried exactly once; the preflight lists every stage with its affected resources; the authorized-key guard shape; recovery classification, budget exhaustion, cancellation, stream/stdin/upload fidelity, structured delegation, and Close semantics each pinned in internal/ssh. AUDIT_OPEN.md gains the C08 programme-slice record, including this session's secrets-argv inventory: the network-join exposure (fixed in the prior commit), the accepted private-file artifacts (backup_alert 0700 script, root-only crontab), and the one remaining exposure — S3Config.AWS inline env-prefix creds in internal/backup — recorded as C07-lane-owned (backup/restore internals), with the C08 remainder list. Gates: go build ./... && go vet ./... clean; go test ./... -count=1 26/26 packages ok (one unrelated timing-flaky admission test passes 3/3 in isolation and on clean HEAD); -race on ssh/cli/network ok; gofmt clean on touched files.
…cutover, RPO/RTO receipts (parallel lane) # Conflicts: # AUDIT_OPEN.md
…ults, bounded lifetime, stdin secrets everywhere, TOFU concurrency, resumable setup (parallel lane) Conflict resolution (backup.go): C07's shared planAccessoryDump planner won structurally, with C08's env-file credential transport folded INTO the planner (planCredential + stage; both callers - AccessoryBackup and the DR bundle dump path - stage before running cmd, so the two paths cannot drift on secret transport either). The mysql RESTORE path keeps C08's inline env-file upload + scrub-on-kept-tmpdir verbatim.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirror sync from Forgejo (7132dfe..d9b652d). Same tree.