From 3381947ebb598886501fd4ac5fa34d842be946c2 Mon Sep 17 00:00:00 2001 From: Tyler <53561637+im-tyler@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:20:35 -0700 Subject: [PATCH 1/5] =?UTF-8?q?feat(state,deploy):=20F16=20=E2=80=94=20own?= =?UTF-8?q?er-token=20fenced=20locks=20with=20renewal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every auto lock now carries a unique owner token (the fencing token) and is renewed in the background every staleLockTTL/3; staleness is measured from the last renewal, so a live-but-slow deploy is never falsely broken (the stranding hazard the register warned about) while a dead holder's lock still self-heals after the historical 30-minute window. Effect sites verify the fence before mutating: deploy/rollback/static check before every effectful phase, and the atomic state commit itself (state.WriteFenced) renames under the guard — a broken holder's late write is refused with ErrFenceLost instead of interleaving with the new holder. Recovery paths (restoring displaced containers, route rollback) are deliberately NOT fenced: refusing to clean up one's own partial effects is how a fencing design strands an app mid-incident. The owner token doubles as the fencing token; a separate monotonic counter adds nothing in this topology (the .lock dir on the target is the single authority — refusal is exactly 'does it still name us'). MockExecutor now evaluates the guard fragment against its recorded file state, so fence tests prove a refused effect never executes. A nil *Lock is the unfenced legacy shape (DeployLocked), kept honest rather than silently faked. TCL-05's containment is subsumed (release still detached/bounded); the caddy file lock stays short-lived and unfenced by design (seconds-held, no long-holder hazard). --- internal/cli/autodeploy_serve.go | 13 +- internal/cli/deploy.go | 15 +- internal/deploy/deploy.go | 135 +++++++++++--- internal/deploy/fence_test.go | 110 +++++++++++ internal/deploy/rollback.go | 48 ++++- internal/deploy/static.go | 74 ++++++-- internal/ssh/mock.go | 59 ++++++ internal/state/lock.go | 301 +++++++++++++++++++++++++++++++ internal/state/lock_test.go | 219 ++++++++++++++++++++++ internal/state/state.go | 79 +++++--- 10 files changed, 974 insertions(+), 79 deletions(-) create mode 100644 internal/deploy/fence_test.go create mode 100644 internal/state/lock.go create mode 100644 internal/state/lock_test.go diff --git a/internal/cli/autodeploy_serve.go b/internal/cli/autodeploy_serve.go index 960fc0a..6ed03fe 100644 --- a/internal/cli/autodeploy_serve.go +++ b/internal/cli/autodeploy_serve.go @@ -295,10 +295,14 @@ func triggerAutoDeploy(ctx context.Context, executor ssh.Executor, app, branch, if err := state.EnsureAppDir(ctx, executor, app); err != nil { return fmt.Errorf("creating app directory: %w", err) } - if err := state.AcquireLock(ctx, executor, app); err != nil { + // Fenced acquisition (F16): the lease spans fetch → build → deploy, and + // the deploy's effect sites verify the fence. + lk, err := state.AcquireLockFenced(ctx, executor, app) + if err != nil { return fmt.Errorf("acquiring deploy lock: %w", err) } - defer state.ReleaseLockDetached(executor, app) + defer state.ReleaseLockFenced(executor, lk, app) + lk.StartRenewal(executor) if _, err := executor.Run(ctx, "mkdir -p "+ssh.ShellQuote(buildDir)); err != nil { return fmt.Errorf("creating build directory: %w", err) @@ -396,6 +400,7 @@ func triggerAutoDeploy(ctx context.Context, executor ssh.Executor, app, branch, // The outer lock taken at the top of triggerAutoDeploy is still held — // route through the locked entry point so Deploy doesn't deadlock on its - // own second acquisition (audit F07). - return deployBuiltImageLockMode(ctx, executor, appCfg, image, version, "localhost", false, needsBuild, true) + // own second acquisition (audit F07), passing the fence handle so the + // deploy's effects stay fenced (F16). + return deployBuiltImageLockMode(ctx, executor, appCfg, image, version, "localhost", false, needsBuild, lk) } diff --git a/internal/cli/deploy.go b/internal/cli/deploy.go index e5542b4..97e5ed8 100644 --- a/internal/cli/deploy.go +++ b/internal/cli/deploy.go @@ -479,14 +479,15 @@ func deployAppConfig(flags *Flags, appCfg *config.AppConfig, serverName, image, // string for the notification payload (a hostname for the SSH path, // "localhost" for the resident-server path). func deployBuiltImage(ctx context.Context, executor ssh.Executor, appCfg *config.AppConfig, image, version, serverDisplay string, migrateVolumes, needsBuild bool) error { - return deployBuiltImageLockMode(ctx, executor, appCfg, image, version, serverDisplay, migrateVolumes, needsBuild, false) + return deployBuiltImageLockMode(ctx, executor, appCfg, image, version, serverDisplay, migrateVolumes, needsBuild, nil) } // deployBuiltImageLockMode is deployBuiltImage with an explicit lock mode: -// lockHeld=true when the caller already owns the app lock (the resident -// autodeploy path, which locks before fetching) and must not let -// Deployer.Deploy acquire it a second time (audit F07). -func deployBuiltImageLockMode(ctx context.Context, executor ssh.Executor, appCfg *config.AppConfig, image, version, serverDisplay string, migrateVolumes, needsBuild, lockHeld bool) error { +// a non-nil lk is a lock the caller already owns (the resident autodeploy +// path, which locks before fetching, and the terminal path's early lease — +// audit F07/F08) and must not let Deployer.Deploy acquire it a second time; +// nil means Deployer.Deploy acquires the lock itself. +func deployBuiltImageLockMode(ctx context.Context, executor ssh.Executor, appCfg *config.AppConfig, image, version, serverDisplay string, migrateVolumes, needsBuild bool, lk *state.Lock) error { appliedManifest, manifestSHA256, err := config.NormalizeAndDigest(appCfg, image) if err != nil { return fmt.Errorf("normalizing applied manifest: %w", err) @@ -644,8 +645,8 @@ func deployBuiltImageLockMode(ctx context.Context, executor ssh.Executor, appCfg multiNotifier := buildNotifier(appCfg) var deployErr error - if lockHeld { - deployErr = deployer.DeployLocked(ctx, deployCfg) + if lk != nil { + deployErr = deployer.DeployFenced(ctx, deployCfg, lk) } else { deployErr = deployer.Deploy(ctx, deployCfg) } diff --git a/internal/deploy/deploy.go b/internal/deploy/deploy.go index 063df5b..f668e47 100644 --- a/internal/deploy/deploy.go +++ b/internal/deploy/deploy.go @@ -142,8 +142,8 @@ func (c Config) validate() error { // Deploy performs a zero-downtime deploy, acquiring the app lock for the // duration. Callers that already hold the app lock (the autodeploy path, // which locks before fetching so the checkout can't race a concurrent -// trigger) must call DeployLocked instead — the mkdir lock is not -// reentrant, so acquiring it twice fails (audit TCL-01). +// trigger) must call DeployFenced with their lock handle instead — the mkdir +// lock is not reentrant, so acquiring it twice fails (audit TCL-01). // // Flow: lock → start web → health check → start workers → route traffic → // write state → stop old containers → log → unlock. @@ -157,17 +157,34 @@ func (d *Deployer) Deploy(ctx context.Context, cfg Config) error { if err := state.EnsureAppDir(ctx, d.exec, cfg.App); err != nil { return fmt.Errorf("creating app directory: %w", err) } - if err := state.AcquireLock(ctx, d.exec, cfg.App); err != nil { + lk, err := state.AcquireLockFenced(ctx, d.exec, cfg.App) + if err != nil { return err } - defer state.ReleaseLockDetached(d.exec, cfg.App) - return d.DeployLocked(ctx, cfg) + defer state.ReleaseLockFenced(d.exec, lk, cfg.App) + // Renewal is what makes the lock's TTL safe for a slow-but-live deploy + // (F16): the heartbeat keeps the lock fresh, so only a dead holder's + // lock is ever broken as stale. + lk.StartRenewal(d.exec) + return d.DeployFenced(ctx, cfg, lk) } // DeployLocked performs a zero-downtime deploy WITHOUT acquiring the app -// lock. The caller must already hold it (see Deploy); this function does not -// reacquire or release it. +// lock and WITHOUT fence checks — the pre-F16 shape, kept for callers and +// tests that hold no lock handle. Production callers that already own the +// lock pass the handle to DeployFenced instead, so their effects stay +// fenced. func (d *Deployer) DeployLocked(ctx context.Context, cfg Config) error { + return d.DeployFenced(ctx, cfg, nil) +} + +// DeployFenced is the deploy body under an explicitly-held lock handle +// (audit F16). lk may be nil (no fencing — see DeployLocked). Fence checks +// precede every effectful phase; recovery paths (restoreDisplacedAndStarted, +// abortStateCommit) are deliberately NOT fenced — refusing to clean up this +// operation's own partial effects is how a fencing design strands an app +// mid-incident. +func (d *Deployer) DeployFenced(ctx context.Context, cfg Config, lk *state.Lock) error { if err := cfg.validate(); err != nil { return err } @@ -303,6 +320,11 @@ func (d *Deployer) DeployLocked(ctx context.Context, cfg Config) error { // F03). sameVersion := current != nil && current.CurrentHash == cfg.Version if sameVersion { + // Fence (F16): the renames below mutate the live workload — a + // holder whose lock was broken must not touch it. + if err := lk.Check(ctx, d.exec); err != nil { + return err + } seen := map[string]bool{} for _, process := range sortedProcessNames(processes) { for ri := 1; ri <= replicas; ri++ { @@ -355,6 +377,12 @@ func (d *Deployer) DeployLocked(ctx context.Context, cfg Config) error { // configuration and fixed port. var displacedHostWeb []string if recreateWeb { + // Fence (F16): stopping the fixed-port workload is the deploy's + // first destructive effect; nothing of ours needs restoring yet, + // so a lost fence is a plain abort. + if err := lk.Check(ctx, d.exec); err != nil { + return err + } names, _ := d.exec.Run(ctx, fmt.Sprintf( "docker ps --filter label=teploy.app=%s --filter label=teploy.process=web --format '{{.Names}}'", cfg.App)) for _, name := range strings.Fields(names) { @@ -413,6 +441,12 @@ func (d *Deployer) DeployLocked(ctx context.Context, cfg Config) error { } // 6. Start web container(s). + // Fence (F16): container creation is an effect. A lost fence here must + // still restore whatever this deploy displaced (recovery is never + // fenced — see DeployFenced's doc). + if err := lk.Check(ctx, d.exec); err != nil { + return restoreDisplacedAndStarted(err) + } for i := 0; i < replicas; i++ { name := docker.ReplicaContainerName(cfg.App, "web", cfg.Version, i+1, replicas) webContainerNames[i] = name @@ -494,6 +528,9 @@ func (d *Deployer) DeployLocked(ctx context.Context, cfg Config) error { } // 10. Start non-web process containers (workers, etc. — no replicas, one each). + if err := lk.Check(ctx, d.exec); err != nil { + return fail(err) + } for _, process := range sortedProcessNames(processes) { if process == "web" { continue @@ -529,6 +566,12 @@ func (d *Deployer) DeployLocked(ctx context.Context, cfg Config) error { // teploy docker network, so Teploy has nothing to do here. if cfg.usesCaddy() { fmt.Fprintln(d.out, "Updating routes...") + // Fence (F16): the route switch commits traffic to this deploy's + // containers; a late write here would hijack a newer operation's + // route. + if err := lk.Check(ctx, d.exec); err != nil { + return fail(err) + } tls := caddy.TLS{Cert: cfg.TLSCert, Key: cfg.TLSKey, Internal: cfg.TLSInternal} if replicas > 1 { upstreams := make([]caddy.Upstream, replicas) @@ -582,7 +625,10 @@ func (d *Deployer) DeployLocked(ctx context.Context, cfg Config) error { newState.PreviousPorts = current.CurrentPorts newState.PreviousHash = current.CurrentHash } - if err := state.Write(ctx, d.exec, cfg.App, newState); err != nil { + // The commit runs under the fence (F16): the atomic rename that makes + // this deploy authoritative is a guarded effect, so a broken holder + // commits nothing. + if err := state.WriteFenced(ctx, d.exec, cfg.App, newState, lk); err != nil { return d.abortStateCommit(ctx, cfg, current, started, displacedHostWeb, start, err) } @@ -605,6 +651,16 @@ func (d *Deployer) DeployLocked(ctx context.Context, cfg Config) error { // listed at snapshot time. if predecessorsListed { for _, ct := range predecessors { + // Fence (F16): the deploy is already committed; a fence loss + // mid-cleanup means another operation owns the app now. Refuse + // further stops (loudly) rather than interleaving with it — + // leaving an old worker running is degraded but visible. + if lk != nil { + if err := lk.Check(ctx, d.exec); err != nil { + fmt.Fprintf(d.out, "Warning: predecessor cleanup stopped — %v\n", err) + break + } + } fmt.Fprintf(d.out, "Stopping old container %s...\n", ct.Name) if err := d.docker.Stop(ctx, ct.Name, stopTimeout); err != nil { // Traffic is already committed to the new generation; a @@ -621,7 +677,17 @@ func (d *Deployer) DeployLocked(ctx context.Context, cfg Config) error { } } } else if current != nil && current.CurrentHash != "" { - stopOldWorkloadsByName(ctx, d.docker, d.out, cfg, current, processes, stopTimeout) + // Fence (F16): same refusal as the snapshot-driven cleanup above — + // post-commit cleanup never interleaves with a new holder. + if lk != nil { + if err := lk.Check(ctx, d.exec); err != nil { + fmt.Fprintf(d.out, "Warning: predecessor cleanup skipped — %v\n", err) + } else { + stopOldWorkloadsByName(ctx, d.docker, d.out, cfg, current, processes, stopTimeout) + } + } else { + stopOldWorkloadsByName(ctx, d.docker, d.out, cfg, current, processes, stopTimeout) + } } // 15. Clean up old bridged assets. @@ -642,26 +708,37 @@ func (d *Deployer) DeployLocked(ctx context.Context, cfg Config) error { // version and the immediately-previous version so a rollback target // is preserved regardless of timestamp ordering. if cfg.KeepVersions > 0 { - var prevHash string - if current != nil { - prevHash = current.CurrentHash - } - // Pinned versions are protected from pruning regardless of the keep - // window (teploy pin). Read them off the server so terminal, dash, - // and autodeploy all honor the same set. A read failure SKIPS - // pruning entirely — treating an unreadable pin file as "no pins" - // could delete versions the operator deliberately retained (F78). - protected := []string{cfg.Version, prevHash} - pins, pinsErr := state.ReadPins(ctx, d.exec, cfg.App) - if pinsErr != nil { - fmt.Fprintf(d.out, "Warning: version prune skipped — pin state could not be read: %v\n", pinsErr) - } else { - protected = append(protected, pins...) - pruned, err := d.docker.PruneVersions(ctx, cfg.App, cfg.KeepVersions, protected...) - if err != nil { - fmt.Fprintf(d.out, "Warning: version prune failed: %v\n", err) - } else if len(pruned) > 0 { - fmt.Fprintf(d.out, "Pruned %d superseded version(s): %s\n", len(pruned), strings.Join(pruned, ", ")) + // Fence (F16): pruning removes other versions' containers; a + // stale holder must not delete under a new owner's feet. + fenceOK := true + if lk != nil { + if err := lk.Check(ctx, d.exec); err != nil { + fmt.Fprintf(d.out, "Warning: version prune skipped — %v\n", err) + fenceOK = false + } + } + if fenceOK { + var prevHash string + if current != nil { + prevHash = current.CurrentHash + } + // Pinned versions are protected from pruning regardless of the keep + // window (teploy pin). Read them off the server so terminal, dash, + // and autodeploy all honor the same set. A read failure SKIPS + // pruning entirely — treating an unreadable pin file as "no pins" + // could delete versions the operator deliberately retained (F78). + protected := []string{cfg.Version, prevHash} + pins, pinsErr := state.ReadPins(ctx, d.exec, cfg.App) + if pinsErr != nil { + fmt.Fprintf(d.out, "Warning: version prune skipped — pin state could not be read: %v\n", pinsErr) + } else { + protected = append(protected, pins...) + pruned, err := d.docker.PruneVersions(ctx, cfg.App, cfg.KeepVersions, protected...) + if err != nil { + fmt.Fprintf(d.out, "Warning: version prune failed: %v\n", err) + } else if len(pruned) > 0 { + fmt.Fprintf(d.out, "Pruned %d superseded version(s): %s\n", len(pruned), strings.Join(pruned, ", ")) + } } } } diff --git a/internal/deploy/fence_test.go b/internal/deploy/fence_test.go new file mode 100644 index 0000000..f712163 --- /dev/null +++ b/internal/deploy/fence_test.go @@ -0,0 +1,110 @@ +package deploy + +import ( + "bytes" + "context" + "errors" + "strings" + "testing" + "time" + + "github.com/useteploy/teploy/internal/ssh" + "github.com/useteploy/teploy/internal/state" +) + +// fenceHappyPathMocks is the minimal successful-deploy mock set (mirrors +// TestDeploy_FirstDeploy) plus the lock acquisition, so a test can hold a +// REAL fenced lock handle and drive DeployFenced with it. +func fenceHappyPathMocks(app string) []ssh.MockCommand { + return []ssh.MockCommand{ + ssh.MockCommand{Match: "mkdir /deployments/" + app + "/.lock", Output: ""}, + ssh.MockCommand{Match: "if [ ! -e '/deployments/" + app + "/state.json' ]", Output: "absent"}, + ssh.MockCommand{Match: "if [ ! -e '/deployments/" + app + "/state' ]", Output: "absent"}, + ssh.MockCommand{Match: "ss -tln", Output: ssOutput}, + ssh.MockCommand{Match: "docker run", Output: "abc123def456"}, + ssh.MockCommand{Match: "docker inspect -f '{{.Image}}'", Output: "sha256:" + strings.Repeat("a", 64)}, + ssh.MockCommand{Match: "docker inspect", Output: "running"}, + ssh.MockCommand{Match: "curl -s -o /dev/null", Output: "200"}, + ssh.MockCommand{Match: "curl -sf http://localhost:2019/config/apps/http/servers/srv0", Output: `{"listen":[":80",":443"]}`}, + ssh.MockCommand{Match: "curl -sf -X PATCH", Err: errors.New("not found")}, + ssh.MockCommand{Match: "curl -sf -X POST http://localhost:2019/config/apps/http/servers/srv0/routes", Output: ""}, + ssh.MockCommand{Match: "rm -f /tmp/teploy_caddy", Output: ""}, + ssh.MockCommand{Match: "cat /deployments/caddy/Caddyfile", Output: "{\n\tadmin 0.0.0.0:2019\n}\n"}, + ssh.MockCommand{Match: "mv /tmp/teploy_caddyfile.tmp", Output: ""}, + ssh.MockCommand{Match: "mkdir /deployments/caddy/.lock", Output: ""}, + ssh.MockCommand{Match: "a=$(docker exec caddy md5sum", Output: "TEPLOY_CADDY_OK"}, + ssh.MockCommand{Match: "docker exec caddy caddy reload", Output: ""}, + ssh.MockCommand{Match: "rmdir /deployments/caddy/.lock", Output: ""}, + ssh.MockCommand{Match: "printf %s", Output: ""}, + ssh.MockCommand{Match: "rm -rf /deployments/" + app + "/.lock", Output: ""}, + } +} + +// TestDeployFenced_HappyPathChecksFence proves the fenced deploy path asks +// the server for holdership before its effectful phases: the grep guard +// commands appear, and with the lock held the deploy succeeds end to end. +func TestDeployFenced_HappyPathChecksFence(t *testing.T) { + app := "fency" + mock := ssh.NewMockExecutor("1.2.3.4", fenceHappyPathMocks(app)...) + lk, err := state.AcquireLockFenced(context.Background(), mock, app) + if err != nil { + t.Fatalf("AcquireLockFenced: %v", err) + } + var buf bytes.Buffer + d := NewDeployer(mock, &buf) + if err := d.DeployFenced(context.Background(), Config{ + App: app, + Domain: "fency.com", + Image: "fency:latest", + Version: "abc123", + Health: HealthConfig{Timeout: 5 * time.Second, Interval: 10 * time.Millisecond}, + }, lk); err != nil { + t.Fatalf("DeployFenced: %v", err) + } + guards := 0 + for _, c := range mock.Calls { + if strings.HasPrefix(c, "grep -q '") && strings.Contains(c, "/.lock/info") { + guards++ + } + } + if guards == 0 { + t.Error("expected fence guard checks against .lock/info during the deploy") + } +} + +// TestDeployFenced_LateHolderRefusedToStartContainers is the F16 core +// safety property: a holder whose lock was broken and re-acquired by +// another operation must have its effects refused — here, before any +// container starts, so nothing is half-applied and nothing was mutated. +func TestDeployFenced_LateHolderRefusedToStartContainers(t *testing.T) { + app := "fency" + mock := ssh.NewMockExecutor("1.2.3.4", fenceHappyPathMocks(app)...) + lk, err := state.AcquireLockFenced(context.Background(), mock, app) + if err != nil { + t.Fatalf("AcquireLockFenced: %v", err) + } + // The lock breaks and a second operation acquires it mid-flight: the + // info file now names the other owner. + mock.Files["/deployments/"+app+"/.lock/info"] = []byte(`{"type":"auto","owner":"secondoperation"}`) + + var buf bytes.Buffer + d := NewDeployer(mock, &buf) + err = d.DeployFenced(context.Background(), Config{ + App: app, + Domain: "fency.com", + Image: "fency:latest", + Version: "abc123", + Health: HealthConfig{Timeout: 5 * time.Second, Interval: 10 * time.Millisecond}, + }, lk) + if !errors.Is(err, state.ErrFenceLost) { + t.Fatalf("expected ErrFenceLost, got %v", err) + } + for _, c := range mock.Calls { + if strings.Contains(c, "docker run") { + t.Errorf("late holder's container start must be refused, saw: %s", c) + } + } + if _, ok := mock.Files["/deployments/"+app+"/state.json"]; ok { + t.Error("late holder must not commit state") + } +} diff --git a/internal/deploy/rollback.go b/internal/deploy/rollback.go index 90d2146..67e5acd 100644 --- a/internal/deploy/rollback.go +++ b/internal/deploy/rollback.go @@ -93,14 +93,17 @@ func Rollback(ctx context.Context, exec ssh.Executor, out io.Writer, cfg Rollbac } // 1. Serialize against deploys and other rollbacks BEFORE reading state - // or resolving the target (F11 + TCL-06). + // or resolving the target (F11 + TCL-06), with the fencing handle so + // effect sites can refuse a broken holder's late writes (F16). if err := state.EnsureAppDir(ctx, exec, cfg.App); err != nil { return fmt.Errorf("creating app directory: %w", err) } - if err := state.AcquireLock(ctx, exec, cfg.App); err != nil { + lk, err := state.AcquireLockFenced(ctx, exec, cfg.App) + if err != nil { return fmt.Errorf("acquiring deploy lock: %w", err) } - defer state.ReleaseLockDetached(exec, cfg.App) + defer state.ReleaseLockFenced(exec, lk, cfg.App) + lk.StartRenewal(exec) // 2. Read state and resolve the rollback target — under the lock. current, err := state.Read(ctx, exec, cfg.App) @@ -231,6 +234,11 @@ func Rollback(ctx context.Context, exec ssh.Executor, out io.Writer, cfg Rollbac } } if fixedPorts { + // Fence (F16): stopping the fixed-port workload is this rollback's + // first destructive effect; nothing of ours needs restoring yet. + if err := lk.Check(ctx, exec); err != nil { + return err + } for _, c := range containers { // Displace only the RUNNING web containers of the AUTHORITATIVE // current generation (TCL-07). The old filter (any non-target @@ -260,6 +268,16 @@ func Rollback(ctx context.Context, exec ssh.Executor, out io.Writer, cfg Rollbac var started []string var targetWeb []docker.Container for _, c := range targetContainers { + // Fence (F16): each target restart is an effect; a lost fence + // unwinds what this rollback started and restores the displaced + // workload before bailing (recovery is never fenced). + if err := lk.Check(ctx, exec); err != nil { + for _, name := range started { + dk.Stop(ctx, name, 5) + } + restoreDisplaced() + return err + } fmt.Fprintf(out, "Starting %s...\n", c.Name) // Recreate rather than `docker start`: Docker 29 silently fails // to re-publish HostConfig.PortBindings on `docker start` when @@ -361,6 +379,15 @@ func Rollback(ctx context.Context, exec ssh.Executor, out io.Writer, cfg Rollbac // container Teploy starts (in this case, the target version's). if cfg.usesCaddy() { fmt.Fprintln(out, "Updating routes...") + // Fence (F16): the route switch commits traffic to the target — + // a late write would hijack a newer operation's route. + if err := lk.Check(ctx, exec); err != nil { + for _, name := range started { + dk.Stop(ctx, name, 5) + } + restoreDisplaced() + return err + } tls := caddy.TLS{Cert: cfg.TLSCert, Key: cfg.TLSKey, Internal: cfg.TLSInternal} // The Caddy upstream port is the recorded primary container port // when there is one (TCL-14); without a record the first exposed @@ -435,7 +462,9 @@ func Rollback(ctx context.Context, exec ssh.Executor, out io.Writer, cfg Rollbac if digest, digestErr := dk.ContainerImageDigest(ctx, targetWeb[0].Name); digestErr == nil { newState.ImageDigest = digest } - if err := state.Write(ctx, exec, cfg.App, newState); err != nil { + // The commit runs under the fence (F16): the atomic rename that makes + // the rollback authoritative is a guarded effect. + if err := state.WriteFenced(ctx, exec, cfg.App, newState, lk); err != nil { // Fixed host ports: the target holds them. Stop it, restore the // displaced workload, then remove the uncommitted target — previously // this branch skipped the restore and still claimed "the original @@ -462,8 +491,17 @@ func Rollback(ctx context.Context, exec ssh.Executor, out io.Writer, cfg Rollbac } // 6. The authoritative commit succeeded; the prior current workload can - // now be stopped (match by version label — see step 2). + // now be stopped (match by version label — see step 2). A fence loss + // here (F16) means another operation owns the app: refuse further + // stops loudly rather than interleave — leaving the superseded workload + // running is degraded but visible. for _, c := range containers { + if lk != nil { + if err := lk.Check(ctx, exec); err != nil { + fmt.Fprintf(out, "Warning: current-workload cleanup stopped — %v\n", err) + break + } + } if c.Labels["teploy.version"] == current.CurrentHash && c.State == "running" { fmt.Fprintf(out, "Stopping %s...\n", c.Name) dk.Stop(ctx, c.Name, stopTimeout) diff --git a/internal/deploy/static.go b/internal/deploy/static.go index 61d6099..c5e3c83 100644 --- a/internal/deploy/static.go +++ b/internal/deploy/static.go @@ -162,14 +162,17 @@ func (d *StaticDeployer) Deploy(ctx context.Context, cfg StaticConfig) error { shortHash := hash[:12] fmt.Fprintf(d.out, " release %s\n", shortHash) - // 4. Lock the app to avoid concurrent deploys racing on the symlink swap. + // 4. Lock the app to avoid concurrent deploys racing on the symlink swap + // (F16: fenced, so a broken holder's late writes are refused). if err := state.EnsureAppDir(ctx, d.exec, cfg.App); err != nil { return fmt.Errorf("ensure app dir: %w", err) } - if err := state.AcquireLock(ctx, d.exec, cfg.App); err != nil { + lk, err := state.AcquireLockFenced(ctx, d.exec, cfg.App) + if err != nil { return fmt.Errorf("acquire lock: %w", err) } - defer state.ReleaseLockDetached(d.exec, cfg.App) + defer state.ReleaseLockFenced(d.exec, lk, cfg.App) + lk.StartRenewal(d.exec) // 5. Read prior state for rollback bookkeeping. A read failure aborts — // treating unreadable state as "no state" would drop the rollback @@ -191,6 +194,12 @@ func (d *StaticDeployer) Deploy(ctx context.Context, cfg StaticConfig) error { return fmt.Errorf("mkdir releases: %w", err) } + // Fence (F16): the upload and everything after it are effects of this + // attempt; check before the first one (nothing has been mutated yet). + if err := lk.Check(ctx, d.exec); err != nil { + return err + } + exists, _ := d.exec.Run(ctx, fmt.Sprintf("test -d %s && echo yes || true", finalRelease)) if strings.TrimSpace(exists) == "yes" { fmt.Fprintf(d.out, " release %s already on server, skipping upload\n", shortHash) @@ -212,13 +221,24 @@ func (d *StaticDeployer) Deploy(ctx context.Context, cfg StaticConfig) error { // 7. Atomically flip the `current` symlink. ln -sfn is NOT atomic // (it unlinks the old link before creating the new one, so a reader // in the gap sees ENOENT); swapCurrentLink creates a sibling link - // and renames it over `current` in one step (audit F53). + // and renames it over `current` in one step (audit F53). Fence- + // checked (F16): the swap is the release-cutover effect. + if err := lk.Check(ctx, d.exec); err != nil { + return err + } if err := d.swapCurrentLink(ctx, cfg.App, cfg.StateDir, shortHash); err != nil { return fmt.Errorf("symlink swap: %w", err) } // 8. Upsert Caddyfile block. The container-side root is the mount path, - // not the host path — Caddy reads through the bind mount. + // not the host path — Caddy reads through the bind mount. Fence-checked + // (F16): the route switch commits traffic to the new release. + if err := lk.Check(ctx, d.exec); err != nil { + if restoreErr := d.restoreStaticLink(ctx, cfg.App, cfg.StateDir, prior); restoreErr != nil { + return fmt.Errorf("fence check: %w; restoring the prior static release failed: %v", err, restoreErr) + } + return fmt.Errorf("fence check: %w; the prior static release was restored", err) + } if err := d.caddy.SetStaticRoute(ctx, cfg.App, cfg.Domain, caddy.StaticBlockOpts{ Root: fmt.Sprintf("%s/%s/current", cfg.MountBase, cfg.App), SPA: cfg.SPA, @@ -246,7 +266,9 @@ func (d *StaticDeployer) Deploy(ctx context.Context, cfg StaticConfig) error { if prior != nil && prior.CurrentHash == shortHash { newState.PreviousRelease = prior.PreviousRelease } - if err := state.Write(ctx, d.exec, cfg.App, newState); err != nil { + // The commit runs under the fence (F16): the rename that makes the new + // release authoritative is a guarded effect. + if err := state.WriteFenced(ctx, d.exec, cfg.App, newState, lk); err != nil { return d.abortStaticStateCommit(ctx, cfg.App, cfg.StateDir, currentLink, prior, err) } @@ -610,10 +632,12 @@ func (d *StaticDeployer) Rollback(ctx context.Context, cfg StaticRollbackConfig) cfg.StateDir = DefaultStateDir } - if err := state.AcquireLock(ctx, d.exec, cfg.App); err != nil { + lk, err := state.AcquireLockFenced(ctx, d.exec, cfg.App) + if err != nil { return fmt.Errorf("acquire lock: %w", err) } - defer state.ReleaseLockDetached(d.exec, cfg.App) + defer state.ReleaseLockFenced(d.exec, lk, cfg.App) + lk.StartRenewal(d.exec) prior, err := state.Read(ctx, d.exec, cfg.App) if err != nil { @@ -653,12 +677,23 @@ func (d *StaticDeployer) Rollback(ctx context.Context, cfg StaticRollbackConfig) return fmt.Errorf("release %s no longer on server (may have been pruned)", target) } + // Fence (F16): the symlink swap is the cutover effect of this rollback. + if err := lk.Check(ctx, d.exec); err != nil { + return err + } if err := d.swapCurrentLink(ctx, cfg.App, cfg.StateDir, target); err != nil { return fmt.Errorf("symlink swap: %w", err) } // Re-assert Caddyfile block so any header/cache changes in the rolled- - // back-from version don't carry over. + // back-from version don't carry over. Fence-checked: the route switch + // commits traffic to the target release. + if err := lk.Check(ctx, d.exec); err != nil { + if restoreErr := d.restoreStaticLink(ctx, cfg.App, cfg.StateDir, prior); restoreErr != nil { + return fmt.Errorf("fence check: %w; restoring release %s failed: %v", err, prior.CurrentHash, restoreErr) + } + return fmt.Errorf("fence check: %w; release %s was restored", err, prior.CurrentHash) + } if err := d.caddy.SetStaticRoute(ctx, cfg.App, cfg.Domain, caddy.StaticBlockOpts{ Root: fmt.Sprintf("%s/%s/current", cfg.MountBase, cfg.App), SPA: cfg.SPA, @@ -685,7 +720,7 @@ func (d *StaticDeployer) Rollback(ctx context.Context, cfg StaticRollbackConfig) if prior.PreviousRelease != nil && prior.PreviousRelease.Hash == target { newState.ApplyRelease(prior.PreviousRelease) } - if err := state.Write(ctx, d.exec, cfg.App, newState); err != nil { + if err := state.WriteFenced(ctx, d.exec, cfg.App, newState, lk); err != nil { if restoreErr := d.restoreStaticLink(ctx, cfg.App, cfg.StateDir, prior); restoreErr != nil { return fmt.Errorf("committing authoritative applied state after static rollback route switch: %w; restoring release %s failed: %v; the target release was left active", err, prior.CurrentHash, restoreErr) } @@ -714,10 +749,12 @@ func (d *StaticDeployer) Rollback(ctx context.Context, cfg StaticRollbackConfig) // tree says nothing about how it was served. No record means asking for one // redeploy, not guessing. func (d *StaticDeployer) RollbackStateOnly(ctx context.Context, app, toHash string) error { - if err := state.AcquireLock(ctx, d.exec, app); err != nil { + lk, err := state.AcquireLockFenced(ctx, d.exec, app) + if err != nil { return fmt.Errorf("acquire lock: %w", err) } - defer state.ReleaseLockDetached(d.exec, app) + defer state.ReleaseLockFenced(d.exec, lk, app) + lk.StartRenewal(d.exec) prior, err := state.Read(ctx, d.exec, app) if err != nil { @@ -746,6 +783,10 @@ func (d *StaticDeployer) RollbackStateOnly(ctx context.Context, app, toHash stri return fmt.Errorf("release %s no longer on server (may have been pruned)", target) } + // Fence (F16): the symlink swap is the cutover effect. + if err := lk.Check(ctx, d.exec); err != nil { + return err + } if err := d.swapCurrentLink(ctx, app, DefaultStateDir, target); err != nil { return fmt.Errorf("symlink swap: %w", err) } @@ -754,6 +795,13 @@ func (d *StaticDeployer) RollbackStateOnly(ctx context.Context, app, toHash stri if domain == "" { domain = prior.Domain } + // Fence-checked: the route switch commits traffic to the target. + if err := lk.Check(ctx, d.exec); err != nil { + if restoreErr := d.restoreStaticLink(ctx, app, DefaultStateDir, prior); restoreErr != nil { + return fmt.Errorf("fence check: %w; restoring release %s failed: %v; the target release was left active", err, prior.CurrentHash, restoreErr) + } + return fmt.Errorf("fence check: %w; release %s was restored", err, prior.CurrentHash) + } if err := d.caddy.SetStaticRoute(ctx, app, domain, caddy.StaticBlockOpts{ Root: fmt.Sprintf("%s/%s/current", DefaultStaticMount, app), SPA: st.SPA, @@ -774,7 +822,7 @@ func (d *StaticDeployer) RollbackStateOnly(ctx context.Context, app, toHash stri if prior.PreviousRelease != nil && prior.PreviousRelease.Hash == target { newState.ApplyRelease(prior.PreviousRelease) } - if err := state.Write(ctx, d.exec, app, newState); err != nil { + if err := state.WriteFenced(ctx, d.exec, app, newState, lk); err != nil { if restoreErr := d.restoreStaticLink(ctx, app, DefaultStateDir, prior); restoreErr != nil { return fmt.Errorf("committing authoritative applied state after static rollback route switch: %w; restoring release %s failed: %v; the target release was left active", err, prior.CurrentHash, restoreErr) } diff --git a/internal/ssh/mock.go b/internal/ssh/mock.go index 9211fb7..34b94d7 100644 --- a/internal/ssh/mock.go +++ b/internal/ssh/mock.go @@ -1,6 +1,7 @@ package ssh import ( + "bytes" "context" "fmt" "io" @@ -43,6 +44,26 @@ func (m *MockExecutor) Run(ctx context.Context, cmd string) (string, error) { m.mu.Lock() m.Calls = append(m.Calls, cmd) + // Fenced-lock guards (internal/state, audit F16) arrive either alone + // (`grep -q '' ''`) or composed with the effect they gate + // (`grep ... || { printf 'TEPLOY_FENCE_LOST\n' >&2; exit 75; }; `). + // Evaluating them against the recorded file state models the server: + // the guard passes while the uploaded lock info still names the owner + // and refuses once it does not, which is what the fence tests need to + // prove a refused effect never executes. + if rest, held, ok := evalFenceGuard(m.Files, cmd); ok { + if !held { + m.mu.Unlock() + return "", fmt.Errorf("exit status 75: TEPLOY_FENCE_LOST") + } + if rest == "" { + m.mu.Unlock() + return "", nil + } + cmd = rest + m.Calls = append(m.Calls, cmd) + } + for i, c := range m.commands { if mockCommandMatches(cmd, c.Match) { if c.Once { @@ -64,6 +85,44 @@ func (m *MockExecutor) Run(ctx context.Context, cmd string) (string, error) { return "", fmt.Errorf("mock: unexpected command: %s", cmd) } +// evalFenceGuard recognizes the guard fragment produced by state.Lock. It +// returns the remaining effect command ("" for a bare guard), whether the +// guard holds against the recorded files, and whether cmd was a guard at +// all. Must be called with m.mu held. +func evalFenceGuard(files map[string][]byte, cmd string) (rest string, held, ok bool) { + const guardSep = " || { printf 'TEPLOY_FENCE_LOST\\n' >&2; exit 75; }; " + if !strings.HasPrefix(cmd, "grep -q ") { + return "", false, false + } + guard, effect := cmd, "" + if i := strings.Index(cmd, guardSep); i >= 0 { + guard, effect = cmd[:i], cmd[i+len(guardSep):] + } + owner, path, parsed := parseFenceGuard(guard) + if !parsed { + return "", false, false + } + data, present := files[path] + held = present && bytes.Contains(data, []byte(owner)) + return effect, held, true +} + +// parseFenceGuard splits `grep -q '' ''` into its two +// single-quoted arguments. +func parseFenceGuard(guard string) (owner, path string, ok bool) { + s := strings.TrimPrefix(guard, "grep -q ") + parts := strings.Split(s, " ") + if len(parts) != 2 { + return "", "", false + } + for _, p := range parts { + if len(p) < 2 || p[0] != '\'' || p[len(p)-1] != '\'' { + return "", "", false + } + } + return parts[0][1 : len(parts[0])-1], parts[1][1 : len(parts[1])-1], true +} + func mockCommandMatches(cmd, match string) bool { if !strings.HasPrefix(cmd, match) { return false diff --git a/internal/state/lock.go b/internal/state/lock.go new file mode 100644 index 0000000..ff1dcfa --- /dev/null +++ b/internal/state/lock.go @@ -0,0 +1,301 @@ +// Fenced deploy locks (audit F16). +// +// The pre-F16 lock was an atomic mkdir plus an info file whose only liveness +// signal was a timestamp: a deploy older than staleLockTTL could be broken by +// the next acquire, and a broken holder that was merely SLOW (not dead) kept +// mutating the server afterwards — its docker runs, state writes, and route +// switches landed on top of whatever the new holder was doing. The register +// warned that a fencing redesign can strand apps mid-incident if it ships +// wrong, so the failure modes here are chosen explicitly: +// +// - Liveness: a holder RENEWS its lock (renew_ts) every renewalInterval; +// a lock only looks stale after staleLockTTL measured from the LAST +// renewal. A live-but-slow deploy is therefore never falsely broken — +// the stranding scenario — while a dead one still self-heals after the +// same 30-minute window as before. +// - Safety: every acquire carries a unique owner token. Effect sites +// verify the token immediately before (and, for single-command effects, +// in the same shell invocation as) the mutation, so a holder whose lock +// was broken or replaced has its late writes REFUSED with +// ErrFenceLost instead of interleaving with the new holder. +// - Recovery is never fenced: cleanup paths that undo this operation's +// own effects (restoring displaced containers, rolling the Caddyfile +// back) still run after fence loss — refusing to CLEAN UP is how a +// fencing design strands an app mid-incident. +// +// The owner token doubles as the fencing token. A separate monotonic +// counter adds nothing in this topology: the .lock directory on the target +// is the single authority (there is no shared resource that could compare +// token ordering independently), and refusal is exactly the test "does the +// authority still name us", which a unique random token answers. + +package state + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strings" + "sync" + "time" + + "github.com/useteploy/teploy/internal/ssh" +) + +// ErrFenceLost is returned when the lock this operation holds no longer +// names it on the server — broken as stale and re-acquired, manually +// unlocked, or its info overwritten. Effects must be refused, not retried: +// another operation owns the app now. +var ErrFenceLost = errors.New("deploy lock fence lost — the lock no longer names this operation on the server; refusing to apply further effects") + +const ( + // fenceLostMarker is echoed to stderr by the guard fragment below so a + // refused effect is identifiable from the (transport-wrapped) error. + fenceLostMarker = "TEPLOY_FENCE_LOST" + + // renewalInterval is how often a held lock is renewed. Three missed + // renewals fit inside staleLockTTL, so a holder that can still talk to + // the server never lets its lock look stale. + renewalInterval = staleLockTTL / 3 +) + +// Lock is a handle to a held deploy lock: the owner token plus the renewal +// state. Acquire it with AcquireLockFenced, start renewal with StartRenewal, +// check effect sites with Check/Guarded, and release with ReleaseLockFenced. +// A nil *Lock is valid and unfenced: every method is a no-op or plain +// passthrough, which keeps the pre-F16 entry points (and their tests) honest +// about doing no fencing rather than silently faking it. +type Lock struct { + app string + owner string + + mu sync.Mutex + lost bool + renewer ssh.Executor + stopRenew chan struct{} + renewDone chan struct{} +} + +// AcquireLockFenced is AcquireLock returning the fence handle. The lock +// itself is identical (same directory, same info schema plus owner); callers +// that ignore the handle get exactly the old behavior. +func AcquireLockFenced(ctx context.Context, exec ssh.Executor, app string) (*Lock, error) { + owner := newOperationID() + if err := acquireAutoLock(ctx, exec, app, owner); err != nil { + return nil, err + } + return &Lock{app: app, owner: owner}, nil +} + +func lockInfoPath(app string) string { + return fmt.Sprintf("%s/%s/.lock/info", deploymentsDir, app) +} + +// Owner returns the lock's owner token (the fencing token). +func (l *Lock) Owner() string { + if l == nil { + return "" + } + return l.owner +} + +// App returns the app the lock was taken for. +func (l *Lock) App() string { + if l == nil { + return "" + } + return l.app +} + +// guardFragment is the shell precondition asserting holdership. Composed +// into the SAME command as the effect it guards, so no interleaving window +// exists between the check and the mutation at the transport granularity. +func (l *Lock) guardFragment() string { + return fmt.Sprintf("grep -q %s %s", ssh.ShellQuote(l.owner), ssh.ShellQuote(lockInfoPath(l.app))) +} + +// Check verifies the server still names this operation as the lock holder. +// Any failure — including transport failure, because an unreachable answer +// cannot prove holdership — reports ErrFenceLost. Call before effectful +// phases; for single-command effects prefer Guarded. +func (l *Lock) Check(ctx context.Context, exec ssh.Executor) error { + if l == nil { + return nil + } + l.mu.Lock() + lost := l.lost + l.mu.Unlock() + if lost { + return ErrFenceLost + } + if _, err := exec.Run(ctx, l.guardFragment()); err != nil { + return fmt.Errorf("%w: lock check failed for %s (%v)", ErrFenceLost, l.app, err) + } + return nil +} + +// Guarded runs effect as a single remote command prefixed by the holdership +// guard: the mutation only executes if the lock still names this operation. +// A refused effect returns an error wrapping ErrFenceLost; any other error is +// the effect's own. +func (l *Lock) Guarded(ctx context.Context, exec ssh.Executor, effect string) (string, error) { + if l == nil { + return exec.Run(ctx, effect) + } + cmd := l.guardFragment() + " || { printf '" + fenceLostMarker + `\n' >&2; exit 75; }; ` + effect + out, err := exec.Run(ctx, cmd) + if err != nil && fenceLostErr(err) { + return "", fmt.Errorf("%w: refusing to run effect for %s", ErrFenceLost, l.app) + } + return out, err +} + +// fenceLostErr reports whether err is the guard refusing the effect (or the +// renewal detecting loss): the stderr marker echoed by the guard fragment, +// or its exit status surfaced by either executor flavor. +func fenceLostErr(err error) bool { + if err == nil { + return false + } + if errors.Is(err, ErrFenceLost) { + return true + } + msg := err.Error() + return strings.Contains(msg, fenceLostMarker) || + strings.Contains(msg, "status 75") || + strings.Contains(msg, "exit status 75") +} + +// StartRenewal begins background renewal of the lock's liveness timestamp. +// Renewal is what makes the TTL safe: a slow-but-alive deploy keeps its lock +// fresh, so only a genuinely dead holder's lock ever looks stale. Proven +// loss (the server no longer names us) stops renewal and poisons the handle; +// transient renewal failures keep trying — the server-side Check remains the +// authority, and a lock that stops being renewed is eventually broken by +// someone else, at which point our own checks refuse. +func (l *Lock) StartRenewal(exec ssh.Executor) { + if l == nil { + return + } + l.mu.Lock() + defer l.mu.Unlock() + if l.stopRenew != nil || l.lost { + return + } + l.renewer = exec + l.stopRenew = make(chan struct{}) + l.renewDone = make(chan struct{}) + // The channels are passed by value: the loop must never re-read the + // fields, because StopRenewal nils them before closing (a select on a + // re-read nil channel sleeps forever). + stop, done := l.stopRenew, l.renewDone + go l.renewLoop(stop, done) +} + +// StopRenewal stops the background renewer and waits for it to exit. +// Idempotent. +func (l *Lock) StopRenewal() { + if l == nil { + return + } + l.mu.Lock() + stop, done := l.stopRenew, l.renewDone + l.stopRenew, l.renewDone = nil, nil + l.mu.Unlock() + if stop != nil { + close(stop) + <-done + } +} + +func (l *Lock) renewLoop(stop, done chan struct{}) { + defer close(done) + ticker := time.NewTicker(renewalInterval) + defer ticker.Stop() + for { + select { + case <-stop: + return + case <-ticker.C: + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + err := l.renew(ctx) + cancel() + if err != nil && fenceLostErr(err) { + // The server proved we are no longer the holder. + l.mu.Lock() + l.lost = true + l.mu.Unlock() + return + } + // Other failures: keep retrying on the next tick. The + // server-side fence check at the next effect boundary is the + // authority on whether to proceed. + } + } +} + +// renew refreshes renew_ts under the holdership guard — a renewal that no +// longer holds must never clobber the new holder's info file. The payload +// is staged to a fixed sibling (writing it is not an effect; the lock dir +// belongs to the holder) and the rename that makes it live is the guarded +// effect, so guard and write cannot interleave. +func (l *Lock) renew(ctx context.Context) error { + l.mu.Lock() + exec := l.renewer + l.mu.Unlock() + if exec == nil { + return nil + } + info := LockInfo{ + Type: "auto", + Owner: l.owner, + TS: time.Now().UTC().Format(time.RFC3339), + RenewTS: time.Now().UTC().Format(time.RFC3339), + } + payload, err := json.Marshal(info) + if err != nil { + return err + } + path := lockInfoPath(l.app) + tmp := path + ".renew" + if err := exec.Upload(ctx, strings.NewReader(string(payload)), tmp, "0644"); err != nil { + return fmt.Errorf("renewing lock info: %w", err) + } + _, err = l.Guarded(ctx, exec, "mv -f -- "+ssh.ShellQuote(tmp)+" "+ssh.ShellQuote(path)) + return err +} + +// ReleaseLockFenced stops renewal and releases the lock. The release itself +// uses the detached bounded context (see ReleaseLockDetached) so a cancelled +// deploy context cannot skip the unlock and strand the app. +func ReleaseLockFenced(exec ssh.Executor, lk *Lock, app string) { + if lk != nil { + lk.StopRenewal() + } + ReleaseLockDetached(exec, app) +} + +// WriteFenced is Write with the state commit under the fence: the content is +// staged to a sibling temp (no effect), and the atomic rename — the instant +// the new state becomes authoritative — runs as a guarded effect. A holder +// that lost the lock commits nothing. +func WriteFenced(ctx context.Context, exec ssh.Executor, app string, s *AppState, lk *Lock) error { + if lk == nil { + return Write(ctx, exec, app, s) + } + data, err := prepareState(s) + if err != nil { + return err + } + path := fmt.Sprintf("%s/%s/state.json", deploymentsDir, app) + tmpPath := path + ".tmp-fence" + if err := exec.Upload(ctx, strings.NewReader(string(data)), tmpPath, "0644"); err != nil { + return fmt.Errorf("uploading temporary state file: %w", err) + } + if _, err := lk.Guarded(ctx, exec, "mv -f -- "+ssh.ShellQuote(tmpPath)+" "+ssh.ShellQuote(path)); err != nil { + // Leave the temp file for diagnosis; it is inert. + return err + } + return nil +} diff --git a/internal/state/lock_test.go b/internal/state/lock_test.go new file mode 100644 index 0000000..17fc04e --- /dev/null +++ b/internal/state/lock_test.go @@ -0,0 +1,219 @@ +package state + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strings" + "testing" + "time" + + "github.com/useteploy/teploy/internal/ssh" +) + +// takeFencedLock acquires a fenced lock on a mock whose mkdir succeeds, and +// returns the handle plus the mock so tests can inspect/corrupt the info +// file the guard reads. Extra commands (e.g. the guarded effect's response) +// are registered up front. +func takeFencedLock(t *testing.T, app string, extra ...ssh.MockCommand) (*Lock, *ssh.MockExecutor) { + t.Helper() + cmds := append([]ssh.MockCommand{ + ssh.MockCommand{Match: "mkdir /deployments/" + app + "/.lock", Output: ""}, + }, extra...) + mock := ssh.NewMockExecutor("1.2.3.4", cmds...) + lk, err := AcquireLockFenced(context.Background(), mock, app) + if err != nil { + t.Fatalf("AcquireLockFenced: %v", err) + } + if _, ok := mock.Files["/deployments/"+app+"/.lock/info"]; !ok { + t.Fatal("lock info not uploaded") + } + return lk, mock +} + +func TestAcquireLockFenced_IssuesOwnerToken(t *testing.T) { + lk, mock := takeFencedLock(t, "myapp") + if lk.Owner() == "" { + t.Fatal("expected a non-empty owner token") + } + var info LockInfo + if err := json.Unmarshal(mock.Files["/deployments/myapp/.lock/info"], &info); err != nil { + t.Fatalf("parsing lock info: %v", err) + } + if info.Owner != lk.Owner() { + t.Errorf("lock info owner %q != handle owner %q", info.Owner, lk.Owner()) + } + if info.Type != "auto" { + t.Errorf("expected auto lock, got %q", info.Type) + } +} + +func TestLock_Check_Held(t *testing.T) { + lk, mock := takeFencedLock(t, "myapp") + if err := lk.Check(context.Background(), mock); err != nil { + t.Fatalf("Check while held: %v", err) + } +} + +func TestLock_Check_LostWhenServerNamesSomeoneElse(t *testing.T) { + lk, mock := takeFencedLock(t, "myapp") + // Simulate the lock being broken and re-acquired by another operation: + // the info file names a different owner. + mock.Files["/deployments/myapp/.lock/info"] = []byte(`{"type":"auto","owner":"someoneelse","ts":"2026-01-01T00:00:00Z"}`) + if err := lk.Check(context.Background(), mock); !errors.Is(err, ErrFenceLost) { + t.Fatalf("expected ErrFenceLost, got %v", err) + } +} + +func TestLock_Check_LostWhenLockVanishes(t *testing.T) { + lk, mock := takeFencedLock(t, "myapp") + delete(mock.Files, "/deployments/myapp/.lock/info") + if err := lk.Check(context.Background(), mock); !errors.Is(err, ErrFenceLost) { + t.Fatalf("expected ErrFenceLost for a vanished lock, got %v", err) + } +} + +func TestLock_Guarded_RunsEffectWhenHeld(t *testing.T) { + lk, mock := takeFencedLock(t, "myapp", ssh.MockCommand{Match: "docker run", Output: "abc123"}) + out, err := lk.Guarded(context.Background(), mock, "docker run --name x") + if err != nil { + t.Fatalf("Guarded: %v", err) + } + if out != "abc123" { + t.Errorf("expected effect output abc123, got %q", out) + } +} + +func TestLock_Guarded_RefusesEffectWhenLost(t *testing.T) { + lk, mock := takeFencedLock(t, "myapp") + mock.Files["/deployments/myapp/.lock/info"] = []byte(`{"type":"auto","owner":"someoneelse"}`) + _, err := lk.Guarded(context.Background(), mock, "docker run --name x") + if !errors.Is(err, ErrFenceLost) { + t.Fatalf("expected ErrFenceLost, got %v", err) + } + for _, c := range mock.Calls { + if strings.HasPrefix(c, "docker run") { + t.Errorf("refused effect must not execute, saw: %s", c) + } + } +} + +func TestLock_Renew_FreshensRenewTS(t *testing.T) { + lk, mock := takeFencedLock(t, "myapp") + lk.StartRenewal(mock) + defer lk.StopRenewal() + if err := lk.renew(context.Background()); err != nil { + t.Fatalf("renew: %v", err) + } + var info LockInfo + if err := json.Unmarshal(mock.Files["/deployments/myapp/.lock/info"], &info); err != nil { + t.Fatalf("parsing renewed info: %v", err) + } + if info.RenewTS == "" { + t.Error("expected renew_ts to be written") + } + if info.Owner != lk.Owner() { + t.Errorf("renewal changed the owner: %q != %q", info.Owner, lk.Owner()) + } +} + +func TestLock_Renew_DetectsLossAndPoisonsHandle(t *testing.T) { + lk, mock := takeFencedLock(t, "myapp") + lk.StartRenewal(mock) + defer lk.StopRenewal() + mock.Files["/deployments/myapp/.lock/info"] = []byte(`{"type":"auto","owner":"someoneelse"}`) + if err := lk.renew(context.Background()); !errors.Is(err, ErrFenceLost) { + t.Fatalf("expected renewal to report ErrFenceLost, got %v", err) + } + // A renewal that no longer holds must not have clobbered the new + // holder's info file. + var info LockInfo + if err := json.Unmarshal(mock.Files["/deployments/myapp/.lock/info"], &info); err != nil { + t.Fatalf("parsing info: %v", err) + } + if info.Owner != "someoneelse" { + t.Errorf("renewal overwrote the new holder's info (owner=%q)", info.Owner) + } +} + +// TestAcquireLock_StaleByRenewTS proves staleness is measured from the last +// renewal (F16): a lock acquired long ago but renewed just now must NOT be +// broken, while the same acquisition age without a renewal must be. +func TestAcquireLock_StaleByRenewTS(t *testing.T) { + old := time.Now().UTC().Add(-2 * staleLockTTL).Format(time.RFC3339) + fresh := time.Now().UTC().Format(time.RFC3339) + + t.Run("renewed lock is fresh", func(t *testing.T) { + mock := ssh.NewMockExecutor("1.2.3.4", + ssh.MockCommand{Match: "mkdir /deployments/myapp/.lock", Err: fmt.Errorf("exists"), Once: true}, + ssh.MockCommand{Match: "cat /deployments/myapp/.lock/info", Output: fmt.Sprintf(`{"type":"auto","owner":"o1","ts":%q,"renew_ts":%q}`, old, fresh)}, + ) + if err := AcquireLock(context.Background(), mock, "myapp"); err == nil || !strings.Contains(err.Error(), "already in progress") { + t.Fatalf("expected 'already in progress' for a renewed lock, got %v", err) + } + for _, c := range mock.Calls { + if strings.HasPrefix(c, "rm -rf") { + t.Errorf("renewed lock must not be broken, saw %s", c) + } + } + }) + + t.Run("unrenewed lock with unparseable renew_ts is stale", func(t *testing.T) { + // An unparseable renew_ts is treated as stale (isStale), so the + // lock is broken and acquisition retried; the retry's mkdir + // succeeds (first attempt fails Once). + mock := ssh.NewMockExecutor("1.2.3.4", + ssh.MockCommand{Match: "mkdir /deployments/myapp/.lock", Err: fmt.Errorf("exists"), Once: true}, + ssh.MockCommand{Match: "mkdir /deployments/myapp/.lock", Output: ""}, + ssh.MockCommand{Match: "cat /deployments/myapp/.lock/info", Output: fmt.Sprintf(`{"type":"auto","ts":%q,"renew_ts":"garbage"}`, old)}, + ) + if err := AcquireLock(context.Background(), mock, "myapp"); err != nil { + t.Fatalf("AcquireLock after breaking the corrupt lock: %v", err) + } + }) +} + +func TestWriteFenced_CommitsUnderFence(t *testing.T) { + lk, mock := takeFencedLock(t, "myapp") + s := &AppState{SchemaVersion: SchemaVersionV2, CurrentHash: "abc", UpdatedAt: time.Now().UTC(), OperationID: "op", Generation: 1} + if err := WriteFenced(context.Background(), mock, "myapp", s, lk); err != nil { + t.Fatalf("WriteFenced: %v", err) + } + data, ok := mock.Files["/deployments/myapp/state.json"] + if !ok { + t.Fatal("state.json not committed") + } + if !strings.Contains(string(data), `"current_hash":"abc"`) { + t.Errorf("unexpected state content: %s", data) + } +} + +func TestWriteFenced_RefusedWhenFenceLost(t *testing.T) { + lk, mock := takeFencedLock(t, "myapp") + mock.Files["/deployments/myapp/.lock/info"] = []byte(`{"type":"auto","owner":"someoneelse"}`) + s := &AppState{SchemaVersion: SchemaVersionV2, CurrentHash: "abc", UpdatedAt: time.Now().UTC(), OperationID: "op", Generation: 1} + err := WriteFenced(context.Background(), mock, "myapp", s, lk) + if !errors.Is(err, ErrFenceLost) { + t.Fatalf("expected ErrFenceLost, got %v", err) + } + if _, ok := mock.Files["/deployments/myapp/state.json"]; ok { + t.Error("a refused fence must not commit state") + } +} + +func TestNilLockIsUnfencedPassthrough(t *testing.T) { + var lk *Lock + mock := ssh.NewMockExecutor("1.2.3.4", + ssh.MockCommand{Match: "echo hi", Output: "hi"}, + ) + if err := lk.Check(context.Background(), mock); err != nil { + t.Fatalf("nil Check must be a no-op, got %v", err) + } + out, err := lk.Guarded(context.Background(), mock, "echo hi") + if err != nil || out != "hi" { + t.Fatalf("nil Guarded must run the effect plainly, got (%q, %v)", out, err) + } + lk.StartRenewal(mock) + lk.StopRenewal() +} diff --git a/internal/state/state.go b/internal/state/state.go index 83f9098..381962a 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -365,14 +365,26 @@ func validateReleaseDigest(release *ReleaseMetadata) error { // key=value file, making that file an import-only migration source rather than // a second writable authority. func Write(ctx context.Context, exec ssh.Executor, app string, s *AppState) error { + data, err := prepareState(s) + if err != nil { + return err + } + path := fmt.Sprintf("%s/%s/state.json", deploymentsDir, app) + return ssh.UploadAtomic(ctx, exec, bytes.NewReader(data), path, "0644") +} + +// prepareState validates and serializes s for writing (shared by Write and +// the fenced commit, audit F16). The trailing newline matches the historical +// format. +func prepareState(s *AppState) ([]byte, error) { if s == nil { - return fmt.Errorf("state is required") + return nil, fmt.Errorf("state is required") } if s.SchemaVersion == 0 { s.SchemaVersion = SchemaVersionV2 } if s.SchemaVersion != SchemaVersionV2 { - return fmt.Errorf("cannot write state schema version %d", s.SchemaVersion) + return nil, fmt.Errorf("cannot write state schema version %d", s.SchemaVersion) } if s.DeploymentType == "" { s.DeploymentType = "container" @@ -390,19 +402,17 @@ func Write(ctx context.Context, exec ssh.Executor, app string, s *AppState) erro s.Generation = 1 } if err := validateManifestDigest(s); err != nil { - return err + return nil, err } if err := validateReleaseDigest(s.PreviousRelease); err != nil { - return fmt.Errorf("validating previous release: %w", err) + return nil, fmt.Errorf("validating previous release: %w", err) } data, err := json.Marshal(s) if err != nil { - return fmt.Errorf("marshaling state: %w", err) + return nil, fmt.Errorf("marshaling state: %w", err) } - data = append(data, '\n') - path := fmt.Sprintf("%s/%s/state.json", deploymentsDir, app) - return ssh.UploadAtomic(ctx, exec, bytes.NewReader(data), path, "0644") + return append(data, '\n'), nil } // LockInfo represents the metadata stored in a .lock directory. @@ -411,6 +421,14 @@ type LockInfo struct { User string `json:"user,omitempty"` Message string `json:"message,omitempty"` TS string `json:"ts"` + // Owner is the unique holder token of an "auto" lock (audit F16) — + // the fencing token effect sites verify. Empty on locks written by + // pre-F16 binaries and on manual/heal locks (never fenced). + Owner string `json:"owner,omitempty"` + // RenewTS is the last renewal heartbeat of an "auto" lock (F16); + // staleness is measured from it when present, so a live-but-slow + // deploy that renews is never broken as stale. + RenewTS string `json:"renew_ts,omitempty"` } // ReadLock reads the lock info for an app. Returns nil if no lock exists. @@ -442,14 +460,23 @@ func (l *LockInfo) IsStale() bool { case "heal": return isHealStale(l.TS) default: - return isStale(l.TS) + return isStale(l.TS, l.RenewTS) } } // AcquireLock acquires the deploy lock for an app using atomic mkdir. // Returns an error if a lock already exists (another deploy in progress or -// manual freeze) and isn't stale (see staleLockTTL). +// manual freeze) and isn't stale (see staleLockTTL). Callers that need the +// fencing handle (audit F16) use AcquireLockFenced; the lock taken is the +// same — this is that call with the handle discarded. func AcquireLock(ctx context.Context, exec ssh.Executor, app string) error { + return acquireAutoLock(ctx, exec, app, newOperationID()) +} + +// acquireAutoLock is the shared "auto" lock acquisition. Every acquire +// carries a unique owner token (F16) so effect sites can refuse a broken +// holder's late writes; the token is opaque to everything pre-F16. +func acquireAutoLock(ctx context.Context, exec ssh.Executor, app, owner string) error { lockPath := fmt.Sprintf("%s/%s/.lock", deploymentsDir, app) if _, err := tryMkdirLock(ctx, exec, lockPath); err != nil { info, _ := ReadLock(ctx, exec, app) @@ -461,7 +488,7 @@ func AcquireLock(ctx context.Context, exec ssh.Executor, app string) error { msg += fmt.Sprintf(". Locked at %s. Use 'teploy unlock' to release.", info.TS) return fmt.Errorf("%s", msg) } - if info != nil && info.Type == "auto" && isStale(info.TS) { + if info != nil && info.Type == "auto" && isStale(info.TS, info.RenewTS) { ReleaseLock(ctx, exec, app) if _, retryErr := tryMkdirLock(ctx, exec, lockPath); retryErr != nil { // Someone else's deploy won the race to re-acquire right @@ -469,7 +496,7 @@ func AcquireLock(ctx context.Context, exec ssh.Executor, app string) error { // normal "in progress" error below. return fmt.Errorf("deploy is already in progress for %s", app) } - return writeLockInfo(ctx, exec, lockPath, app) + return writeLockInfo(ctx, exec, lockPath, app, owner) } // A crashed heal can leave its short-lived "heal" lock behind. A deploy // (authoritative) may break a STALE heal lock so it isn't blocked — but @@ -481,21 +508,22 @@ func AcquireLock(ctx context.Context, exec ssh.Executor, app string) error { if _, retryErr := tryMkdirLock(ctx, exec, lockPath); retryErr != nil { return fmt.Errorf("deploy is already in progress for %s", app) } - return writeLockInfo(ctx, exec, lockPath, app) + return writeLockInfo(ctx, exec, lockPath, app, owner) } return fmt.Errorf("deploy is already in progress for %s", app) } - return writeLockInfo(ctx, exec, lockPath, app) + return writeLockInfo(ctx, exec, lockPath, app, owner) } func tryMkdirLock(ctx context.Context, exec ssh.Executor, lockPath string) (string, error) { return exec.Run(ctx, fmt.Sprintf("mkdir %s 2>/dev/null", lockPath)) } -func writeLockInfo(ctx context.Context, exec ssh.Executor, lockPath, app string) error { +func writeLockInfo(ctx context.Context, exec ssh.Executor, lockPath, app, owner string) error { info, _ := json.Marshal(LockInfo{ - Type: "auto", - TS: time.Now().UTC().Format(time.RFC3339), + Type: "auto", + Owner: owner, + TS: time.Now().UTC().Format(time.RFC3339), }) if err := exec.Upload(ctx, bytes.NewReader(info), lockPath+"/info", "0644"); err != nil { // Lock directory was created but info file failed — release and return error. @@ -505,10 +533,19 @@ func writeLockInfo(ctx context.Context, exec ssh.Executor, lockPath, app string) return nil } -// isStale reports whether an "auto" lock's timestamp is older than -// staleLockTTL. An unparseable timestamp is treated as stale — a lock file -// too corrupted to read its own age isn't one worth respecting. -func isStale(ts string) bool { +// isStale reports whether an "auto" lock is older than staleLockTTL, +// measured from the last RENEWAL when the lock carries one (F16) — a +// live-but-slow deploy renews, so it is never falsely broken — and from the +// acquisition timestamp otherwise (pre-F16 locks). An unparseable timestamp +// is treated as stale — a lock file too corrupted to read its own age isn't +// one worth respecting. +func isStale(ts, renewTS string) bool { + if renewTS != "" { + if parsed, err := time.Parse(time.RFC3339, renewTS); err == nil { + return time.Since(parsed) > staleLockTTL + } + return true + } parsed, err := time.Parse(time.RFC3339, ts) if err != nil { return true From fd93d7aaa9e0d0ef355fd727b9a3012b4e489164 Mon Sep 17 00:00:00 2001 From: Tyler <53561637+im-tyler@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:28:34 -0700 Subject: [PATCH 2/5] =?UTF-8?q?feat(releasemeta,cli,deploy):=20F08=20?= =?UTF-8?q?=E2=80=94=20attempt-scoped=20immutable=20artifacts=20under=20th?= =?UTF-8?q?e=20deploy=20lease?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three artifacts a deploy attempt generates were shared per-app paths: the build context (/deployments//build, rsynced before the app lock was ever taken), the resolved env file (.deploy-env), and the TLS cert/key (/deployments/caddy/tls/.crt). Racing attempts interleaved writes on all three, and the F14 record's env-file reference named a path a later attempt would overwrite. Every attempt now mints (app, hash, random id) and writes its artifacts immutable, in the releasemeta namespace: - build context: /deployments//meta/att/./build, with the previous attempt's build dir as rsync --link-dest so a fresh directory still transfers incrementally and hardlink-shares unchanged files - env file: meta/att/./env — the record's EnvFiles now names bytes no later attempt can touch - TLS: /deployments/caddy/tls/att/./ (container path /etc/caddy/tls/att/…), deliberately kept under the caddy tls dir — the one mount every custom-TLS server provably has; written atomically The terminal deploy path acquires the fenced lease BEFORE artifact generation (the autodeploy path already locked before fetch), so attempts serialize at the source; attempt-scoped paths make the immutability structural even without the lease (teploy build, lockless by design, now builds into its own attempt dir and cannot interleave with a deploy's rsync). Retention: PruneAttempts runs after a committed deploy with the same protection window as version pruning (current + previous + pinned releases; unparsable entries kept, F78 parity). TLS nil-attempt callers (rollback, LB upload) keep the legacy shared paths — pre-F14 records still reference them and F14-recorded releases override from the record. --- internal/build/sync.go | 9 ++ internal/cli/attempt_test.go | 83 +++++++++++ internal/cli/autodeploy_serve.go | 10 +- internal/cli/build.go | 12 +- internal/cli/deploy.go | 115 +++++++++++---- internal/cli/envfile.go | 22 ++- internal/cli/envfile_test.go | 36 +++-- internal/cli/rollback.go | 5 +- internal/cli/scale.go | 10 +- internal/cli/singledeploy.go | 18 ++- internal/deploy/deploy.go | 25 ++++ internal/deploy/fence_test.go | 49 +++++++ internal/releasemeta/attempt.go | 203 +++++++++++++++++++++++++++ internal/releasemeta/attempt_test.go | 121 ++++++++++++++++ 14 files changed, 654 insertions(+), 64 deletions(-) create mode 100644 internal/cli/attempt_test.go create mode 100644 internal/releasemeta/attempt.go create mode 100644 internal/releasemeta/attempt_test.go diff --git a/internal/build/sync.go b/internal/build/sync.go index 4a83d4b..57bea00 100644 --- a/internal/build/sync.go +++ b/internal/build/sync.go @@ -19,6 +19,12 @@ type SyncConfig struct { KeyPath string // SSH key path (optional) Excludes []string // patterns to exclude AcceptNewHost bool // mirror the control connection's --accept-new policy (see Sync) + // LinkDest is an optional remote basis directory for --link-dest: the + // F08 attempt-scoped build contexts are fresh per attempt, so without + // a basis every deploy would re-transfer the whole tree. Pointing at + // the previous attempt's build dir restores incremental transfer and + // hardlink-shares unchanged files (no extra disk per attempt). + LinkDest string } // Sync transfers the local directory to the remote server via rsync over SSH. @@ -45,6 +51,9 @@ func Sync(ctx context.Context, cfg SyncConfig, stdout, stderr io.Writer) error { for _, pattern := range cfg.Excludes { args = append(args, "--exclude", pattern) } + if cfg.LinkDest != "" { + args = append(args, "--link-dest="+cfg.LinkDest) + } remote := ssh.RsyncTarget(cfg.User, cfg.Host, cfg.RemoteDir) args = append(args, localDir, remote) diff --git a/internal/cli/attempt_test.go b/internal/cli/attempt_test.go new file mode 100644 index 0000000..18a06ac --- /dev/null +++ b/internal/cli/attempt_test.go @@ -0,0 +1,83 @@ +package cli + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/useteploy/teploy/internal/config" + "github.com/useteploy/teploy/internal/releasemeta" + "github.com/useteploy/teploy/internal/ssh" +) + +// TestUploadAppTLS_AttemptScoped proves F08's TLS artifact rule: a deploy's +// cert/key land in the attempt's own directory (never the shared legacy +// path), written atomically, and the returned paths are the CONTAINER-side +// ones the Caddyfile references. +func TestUploadAppTLS_AttemptScoped(t *testing.T) { + dir := t.TempDir() + certPath := filepath.Join(dir, "cert.pem") + keyPath := filepath.Join(dir, "key.pem") + if err := os.WriteFile(certPath, []byte("CERT"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(keyPath, []byte("KEY"), 0600); err != nil { + t.Fatal(err) + } + mock := ssh.NewMockExecutor("1.2.3.4", + ssh.MockCommand{Match: "mkdir -p ", Output: ""}, + ssh.MockCommand{Match: "mv -f -- ", Output: ""}, + ) + att := releasemeta.MustAttempt("myapp", "abc123") + cert, key, err := uploadAppTLS(context.Background(), mock, "myapp", &config.TLSConfig{Cert: certPath, Key: keyPath}, &att) + if err != nil { + t.Fatalf("uploadAppTLS: %v", err) + } + if want := "/etc/caddy/tls/att/" + att.Name() + "/myapp.crt"; cert != want { + t.Errorf("cert container path: got %s want %s", cert, want) + } + if want := "/etc/caddy/tls/att/" + att.Name() + "/myapp.key"; key != want { + t.Errorf("key container path: got %s want %s", key, want) + } + hostCert := att.TLSDir() + "/myapp.crt" + data, ok := mock.Files[hostCert] + if !ok || string(data) != "CERT" { + t.Errorf("cert not uploaded to the attempt dir %s (found=%v)", hostCert, ok) + } + if _, legacy := mock.Files["/deployments/caddy/tls/myapp.crt"]; legacy { + t.Error("attempt-scoped upload must not write the legacy shared cert path") + } + for _, c := range mock.Calls { + if strings.HasPrefix(c, "UPLOAD:") && strings.Contains(c, ".key") && strings.Contains(c, "0600") == false { + t.Errorf("key upload mode: %s", c) + } + } +} + +// TestUploadAppTLS_LegacyFallbackForRollback: the rollback CLI passes a nil +// attempt (target hash unknown before the record is read); that path keeps +// the pre-F08 shared layout, which pre-F14 records still reference. +func TestUploadAppTLS_LegacyFallbackForRollback(t *testing.T) { + dir := t.TempDir() + certPath := filepath.Join(dir, "cert.pem") + keyPath := filepath.Join(dir, "key.pem") + if err := os.WriteFile(certPath, []byte("CERT"), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(keyPath, []byte("KEY"), 0600); err != nil { + t.Fatal(err) + } + mock := ssh.NewMockExecutor("1.2.3.4", + ssh.MockCommand{Match: "mkdir -p /deployments/caddy/tls", Output: ""}, + ssh.MockCommand{Match: "mv -f -- ", Output: ""}, + ) + cert, key, err := uploadAppTLS(context.Background(), mock, "myapp", &config.TLSConfig{Cert: certPath, Key: keyPath}, nil) + if err != nil { + t.Fatalf("uploadAppTLS legacy: %v", err) + } + if cert != "/etc/caddy/tls/myapp.crt" || key != "/etc/caddy/tls/myapp.key" { + t.Errorf("legacy paths: got %s / %s", cert, key) + } +} diff --git a/internal/cli/autodeploy_serve.go b/internal/cli/autodeploy_serve.go index 6ed03fe..631422c 100644 --- a/internal/cli/autodeploy_serve.go +++ b/internal/cli/autodeploy_serve.go @@ -18,6 +18,7 @@ import ( "github.com/useteploy/teploy/internal/config" "github.com/useteploy/teploy/internal/docker" "github.com/useteploy/teploy/internal/env" + "github.com/useteploy/teploy/internal/releasemeta" "github.com/useteploy/teploy/internal/ssh" "github.com/useteploy/teploy/internal/state" ) @@ -399,8 +400,9 @@ func triggerAutoDeploy(ctx context.Context, executor ssh.Executor, app, branch, } // The outer lock taken at the top of triggerAutoDeploy is still held — - // route through the locked entry point so Deploy doesn't deadlock on its - // own second acquisition (audit F07), passing the fence handle so the - // deploy's effects stay fenced (F16). - return deployBuiltImageLockMode(ctx, executor, appCfg, image, version, "localhost", false, needsBuild, lk) + // route through the fenced entry point so Deploy doesn't deadlock on its + // own second acquisition (audit F07), passing the fence handle (F16) and + // the attempt that keys this deploy's env/TLS artifacts (F08). + att := releasemeta.MustAttempt(app, version) + return deployBuiltImageFenced(ctx, executor, appCfg, image, version, "localhost", false, needsBuild, lk, &att) } diff --git a/internal/cli/build.go b/internal/cli/build.go index 62e6638..880a8c7 100644 --- a/internal/cli/build.go +++ b/internal/cli/build.go @@ -12,6 +12,7 @@ import ( "github.com/useteploy/teploy/internal/build" "github.com/useteploy/teploy/internal/config" "github.com/useteploy/teploy/internal/docker" + "github.com/useteploy/teploy/internal/releasemeta" "github.com/useteploy/teploy/internal/ssh" ) @@ -168,7 +169,15 @@ func runBuild(flags *Flags, version, destination string) error { return reportBuild(flags, image, version, true) } - remoteDir := fmt.Sprintf("/deployments/%s/build", appCfg.App) + // Attempt-scoped build context (F08): `teploy build` takes no app + // lock, so building into the shared /deployments//build could + // interleave with a concurrent deploy's rsync. A fresh attempt + // directory per run cannot collide with anything; the previous + // attempt's build dir is the --link-dest basis so transfer stays + // incremental. The directory is scratch — the next deploy of this app + // prunes it once its hash leaves the protection window. + att := releasemeta.MustAttempt(appCfg.App, version) + remoteDir := att.BuildDir() if _, err := executor.Run(ctx, "mkdir -p "+remoteDir); err != nil { return fmt.Errorf("creating build directory: %w", err) } @@ -180,6 +189,7 @@ func runBuild(flags *Flags, version, destination string) error { User: user, KeyPath: key, Excludes: build.LoadIgnore("."), + LinkDest: releasemeta.PreviousAttemptBuildDir(ctx, executor, appCfg.App, att.ID), }, out, os.Stderr); err != nil { return fmt.Errorf("syncing source: %w", err) } diff --git a/internal/cli/deploy.go b/internal/cli/deploy.go index 97e5ed8..e543a21 100644 --- a/internal/cli/deploy.go +++ b/internal/cli/deploy.go @@ -26,6 +26,7 @@ import ( "github.com/useteploy/teploy/internal/env" "github.com/useteploy/teploy/internal/multideploy" "github.com/useteploy/teploy/internal/notify" + "github.com/useteploy/teploy/internal/releasemeta" "github.com/useteploy/teploy/internal/secret" "github.com/useteploy/teploy/internal/ssh" "github.com/useteploy/teploy/internal/state" @@ -375,6 +376,27 @@ func deployAppConfig(flags *Flags, appCfg *config.AppConfig, serverName, image, } defer executor.Close() + // 6a. Take the deploy lease NOW, before any artifact is generated + // (F08): the attempt's build context, env file, and TLS cert/key are + // written under this lease, so concurrent attempts of the same app + // serialize at the source instead of interleaving writes onto shared + // per-app paths. The lease is fenced (F16) and renewed in the + // background; it is released when deployAppConfig returns. + if err := state.EnsureAppDir(ctx, executor, appCfg.App); err != nil { + return fmt.Errorf("creating app directory: %w", err) + } + lk, err := state.AcquireLockFenced(ctx, executor, appCfg.App) + if err != nil { + return err + } + defer state.ReleaseLockFenced(executor, lk, appCfg.App) + lk.StartRenewal(executor) + + // The attempt keys every artifact this deploy generates (F08): + // immutable per (release, attempt), so the F14 record's references + // name exactly the bytes that were deployed. + att := releasemeta.MustAttempt(appCfg.App, version) + // 6b. Ensure the pre-built image is available (CI pipeline mode). An image // already present on the server — built or `docker load`ed out of band, and // possibly in no registry at all — must not be re-pulled, or the deploy @@ -408,8 +430,12 @@ func deployAppConfig(flags *Flags, appCfg *config.AppConfig, serverName, image, return fmt.Errorf("local build: %w", err) } } else { - // Server build mode: rsync + build on server. - remoteDir := fmt.Sprintf("/deployments/%s/build", appCfg.App) + // Server build mode: rsync into the ATTEMPT's build context + // (F08) — a directory no other attempt writes — with the + // previous attempt's build dir as rsync's --link-dest basis so + // the fresh directory still transfers incrementally and + // hardlink-shares unchanged files. + remoteDir := att.BuildDir() if _, err := executor.Run(ctx, "mkdir -p "+remoteDir); err != nil { return fmt.Errorf("creating build directory: %w", err) } @@ -423,6 +449,7 @@ func deployAppConfig(flags *Flags, appCfg *config.AppConfig, serverName, image, User: user, KeyPath: key, Excludes: excludes, + LinkDest: releasemeta.PreviousAttemptBuildDir(ctx, executor, appCfg.App, att.ID), }, os.Stdout, os.Stderr); err != nil { return fmt.Errorf("syncing source: %w", err) } @@ -461,7 +488,7 @@ func deployAppConfig(flags *Flags, appCfg *config.AppConfig, serverName, image, } } - return deployBuiltImage(ctx, executor, appCfg, image, version, host, migrateVolumes, needsBuild) + return deployBuiltImageFenced(ctx, executor, appCfg, image, version, host, migrateVolumes, needsBuild, lk, &att) } // deployBuiltImage runs the shared post-build deploy orchestration: @@ -479,15 +506,19 @@ func deployAppConfig(flags *Flags, appCfg *config.AppConfig, serverName, image, // string for the notification payload (a hostname for the SSH path, // "localhost" for the resident-server path). func deployBuiltImage(ctx context.Context, executor ssh.Executor, appCfg *config.AppConfig, image, version, serverDisplay string, migrateVolumes, needsBuild bool) error { - return deployBuiltImageLockMode(ctx, executor, appCfg, image, version, serverDisplay, migrateVolumes, needsBuild, nil) + return deployBuiltImageFenced(ctx, executor, appCfg, image, version, serverDisplay, migrateVolumes, needsBuild, nil, nil) } -// deployBuiltImageLockMode is deployBuiltImage with an explicit lock mode: -// a non-nil lk is a lock the caller already owns (the resident autodeploy -// path, which locks before fetching, and the terminal path's early lease — -// audit F07/F08) and must not let Deployer.Deploy acquire it a second time; -// nil means Deployer.Deploy acquires the lock itself. -func deployBuiltImageLockMode(ctx context.Context, executor ssh.Executor, appCfg *config.AppConfig, image, version, serverDisplay string, migrateVolumes, needsBuild bool, lk *state.Lock) error { +// deployBuiltImageFenced is deployBuiltImage with the caller's lease and +// attempt: lk is a lock the caller already owns (the terminal path's early +// lease and the resident autodeploy path — audits F07/F08) and att keys the +// attempt-scoped artifacts (env file, TLS). lk == nil means Deployer.Deploy +// acquires the lock itself (att must still be non-nil for the env file). +func deployBuiltImageFenced(ctx context.Context, executor ssh.Executor, appCfg *config.AppConfig, image, version, serverDisplay string, migrateVolumes, needsBuild bool, lk *state.Lock, att *releasemeta.Attempt) error { + if att == nil { + attVal := releasemeta.MustAttempt(appCfg.App, version) + att = &attVal + } appliedManifest, manifestSHA256, err := config.NormalizeAndDigest(appCfg, image) if err != nil { return fmt.Errorf("normalizing applied manifest: %w", err) @@ -581,7 +612,9 @@ func deployBuiltImageLockMode(ctx context.Context, executor ssh.Executor, appCfg // 10b. Upload custom TLS cert (if configured) so Caddy can terminate // HTTPS with it instead of ACME — required behind Cloudflare proxy/Tunnel. - tlsCert, tlsKey, tlsInternal, err := resolveAppTLS(ctx, executor, appCfg) + // Attempt-scoped (F08): the cert bytes this deploy references are + // immutable for the release. + tlsCert, tlsKey, tlsInternal, err := resolveAppTLS(ctx, executor, appCfg, att) if err != nil { return err } @@ -590,9 +623,9 @@ func deployBuiltImageLockMode(ctx context.Context, executor ssh.Executor, appCfg } // Container env: teploy.yml's `env:` block plus decrypted secrets, - // uploaded to a fresh env file rather than passed as `docker run -e` - // args — see buildContainerEnvFiles for why. - envFiles, err := buildContainerEnvFiles(ctx, executor, appCfg.App, envFile, appCfg.Env, nil, deploySecrets) + // uploaded to the ATTEMPT's env file (F08) rather than passed as + // `docker run -e` args — see buildContainerEnvFiles for why. + envFiles, err := buildContainerEnvFiles(ctx, executor, appCfg.App, att, envFile, appCfg.Env, nil, deploySecrets) if err != nil { return err } @@ -1122,18 +1155,29 @@ func runStaticDeploy(cfg *config.AppConfig, host, user, key string) error { return nil } -// appTLSContainerPaths returns the container-side cert/key paths for an app's -// custom TLS certificate. These live under /etc/caddy/tls, which the Caddy -// container sees via the /deployments/caddy directory mount. +// appTLSContainerPaths returns the LEGACY container-side cert/key paths for +// an app's custom TLS certificate (pre-F08 layout). Kept because records +// written before attempt-scoped TLS name these paths — a rollback target's +// recorded TLSCert still points here, and those files were never moved. func appTLSContainerPaths(app string) (cert, key string) { return "/etc/caddy/tls/" + app + ".crt", "/etc/caddy/tls/" + app + ".key" } // uploadAppTLS reads the local cert + key referenced by the app's tls config -// and uploads them to the server's /deployments/caddy/tls directory (key mode -// 0600), where the directory-mounted Caddy container reads them. It returns -// the container-side paths to reference in the Caddy site block. -func uploadAppTLS(ctx context.Context, exec ssh.Executor, app string, tls *config.TLSConfig) (cert, key string, err error) { +// and uploads them to the server's attempt-scoped TLS directory (F08: +// /deployments/caddy/tls/att/./, key mode 0600), where the +// directory-mounted Caddy container reads them at /etc/caddy/tls/att/…. +// Attempt-scoping keeps the cert/key immutable for the release that +// references it: the F14 record names these exact bytes, and a concurrent +// or later attempt cannot overwrite them. It returns the container-side +// paths to reference in the Caddy site block. +// +// A nil attempt selects the LEGACY shared path — the pre-F08 layout. That +// is the rollback CLI's fallback: it re-uploads the operator's current +// cert before the rollback target is known, and for any release recorded +// by F14 the record overrides these paths with the target's own attempt +// paths anyway; only backfilled (pre-F14) releases fall back to them. +func uploadAppTLS(ctx context.Context, exec ssh.Executor, app string, tls *config.TLSConfig, att *releasemeta.Attempt) (cert, key string, err error) { certBytes, err := os.ReadFile(tls.Cert) if err != nil { return "", "", fmt.Errorf("reading tls cert %s: %w", tls.Cert, err) @@ -1142,18 +1186,26 @@ func uploadAppTLS(ctx context.Context, exec ssh.Executor, app string, tls *confi if err != nil { return "", "", fmt.Errorf("reading tls key %s: %w", tls.Key, err) } - if _, err := exec.Run(ctx, "mkdir -p /deployments/caddy/tls"); err != nil { - return "", "", fmt.Errorf("creating tls dir: %w", err) + var hostCert, hostKey string + if att != nil { + if _, err := exec.Run(ctx, "mkdir -p "+ssh.ShellQuote(att.TLSDir())); err != nil { + return "", "", fmt.Errorf("creating tls dir: %w", err) + } + hostCert, hostKey = att.TLSDir()+"/"+app+".crt", att.TLSDir()+"/"+app+".key" + cert, key = att.TLSCertPath(), att.TLSKeyPath() + } else { + if _, err := exec.Run(ctx, "mkdir -p /deployments/caddy/tls"); err != nil { + return "", "", fmt.Errorf("creating tls dir: %w", err) + } + hostCert, hostKey = "/deployments/caddy/tls/"+app+".crt", "/deployments/caddy/tls/"+app+".key" + cert, key = "/etc/caddy/tls/"+app+".crt", "/etc/caddy/tls/"+app+".key" } - hostCert := "/deployments/caddy/tls/" + app + ".crt" - hostKey := "/deployments/caddy/tls/" + app + ".key" - if err := exec.Upload(ctx, bytes.NewReader(certBytes), hostCert, "0644"); err != nil { + if err := ssh.UploadAtomic(ctx, exec, bytes.NewReader(certBytes), hostCert, "0644"); err != nil { return "", "", fmt.Errorf("uploading tls cert: %w", err) } - if err := exec.Upload(ctx, bytes.NewReader(keyBytes), hostKey, "0600"); err != nil { + if err := ssh.UploadAtomic(ctx, exec, bytes.NewReader(keyBytes), hostKey, "0600"); err != nil { return "", "", fmt.Errorf("uploading tls key: %w", err) } - cert, key = appTLSContainerPaths(app) return cert, key, nil } @@ -1161,15 +1213,16 @@ func uploadAppTLS(ctx context.Context, exec ssh.Executor, app string, tls *confi // whether tls.internal was requested, so every deploy/rollback call site // can populate deploy.Config's (or RollbackConfig's) TLSCert/TLSKey/ // TLSInternal fields with one call instead of repeating the appCfg.TLS != -// nil / .Internal branch five times. -func resolveAppTLS(ctx context.Context, exec ssh.Executor, appCfg *config.AppConfig) (cert, key string, internal bool, err error) { +// nil / .Internal branch five times. att nil = legacy shared paths (see +// uploadAppTLS). +func resolveAppTLS(ctx context.Context, exec ssh.Executor, appCfg *config.AppConfig, att *releasemeta.Attempt) (cert, key string, internal bool, err error) { if appCfg.TLS == nil { return "", "", false, nil } if appCfg.TLS.Internal { return "", "", true, nil } - cert, key, err = uploadAppTLS(ctx, exec, appCfg.App, appCfg.TLS) + cert, key, err = uploadAppTLS(ctx, exec, appCfg.App, appCfg.TLS, att) return cert, key, false, err } diff --git a/internal/cli/envfile.go b/internal/cli/envfile.go index cbba6c7..c262712 100644 --- a/internal/cli/envfile.go +++ b/internal/cli/envfile.go @@ -8,6 +8,7 @@ import ( "sort" "strings" + "github.com/useteploy/teploy/internal/releasemeta" "github.com/useteploy/teploy/internal/ssh" ) @@ -32,9 +33,10 @@ func expandEnvTemplates(env map[string]string) { // buildContainerEnvFiles computes the full container environment — values // already resolved (YAML templates expanded once by expandEnvTemplates, // env_files loaded literally, decrypted secrets overlaid last so a secret -// always wins over a plaintext default) — and uploads it to a fresh -// per-deploy env file instead of returning it for use as `docker run -e` -// arguments. +// always wins over a plaintext default) — and uploads it to the ATTEMPT's +// env file (F08: /deployments//meta/att/./env, written once +// by this attempt and never rewritten by a later one) instead of returning +// it for use as `docker run -e` arguments. // // This exists because `-e KEY=value` arguments are visible in this host's // `ps aux` / /proc//cmdline output for the life of the `docker run` @@ -48,9 +50,9 @@ func expandEnvTemplates(env map[string]string) { // // Returns the ordered --env-file path list for deploy.Config.EnvFiles: the // existing persisted /deployments//.env first (if present, managed by -// `teploy env set`), then this fresh file last so its values — including -// secrets — take precedence for any overlapping key. -func buildContainerEnvFiles(ctx context.Context, executor ssh.Executor, app, persistedEnvFile string, appEnv, extra, secrets map[string]string) ([]string, error) { +// `teploy env set`), then this attempt's file last so its values — +// including secrets — take precedence for any overlapping key. +func buildContainerEnvFiles(ctx context.Context, executor ssh.Executor, app string, att *releasemeta.Attempt, persistedEnvFile string, appEnv, extra, secrets map[string]string) ([]string, error) { merged := make(map[string]string, len(appEnv)+len(extra)+len(secrets)) for k, v := range appEnv { merged[k] = v @@ -100,7 +102,13 @@ func buildContainerEnvFiles(ctx context.Context, executor ssh.Executor, app, per fmt.Fprintf(&sb, "%s=%s\n", k, merged[k]) } - path := fmt.Sprintf("/deployments/%s/.deploy-env", app) + if att == nil { + return nil, fmt.Errorf("buildContainerEnvFiles requires a deploy attempt (F08) — the env file is attempt-scoped") + } + path := att.EnvFile() + if _, err := executor.Run(ctx, "mkdir -p "+ssh.ShellQuote(att.Dir())); err != nil { + return nil, fmt.Errorf("creating attempt directory: %w", err) + } if err := executor.Upload(ctx, strings.NewReader(sb.String()), path, "0600"); err != nil { return nil, fmt.Errorf("uploading deploy env file: %w", err) } diff --git a/internal/cli/envfile_test.go b/internal/cli/envfile_test.go index a97ceeb..3d87b98 100644 --- a/internal/cli/envfile_test.go +++ b/internal/cli/envfile_test.go @@ -5,9 +5,21 @@ import ( "strings" "testing" + "github.com/useteploy/teploy/internal/releasemeta" "github.com/useteploy/teploy/internal/ssh" ) +// attMock builds a mock that answers the attempt-directory mkdir, plus the +// attempt every env-file test deploys under (F08: the env file is +// attempt-scoped). +func attMock(cmds ...ssh.MockCommand) (*ssh.MockExecutor, *releasemeta.Attempt) { + mock := ssh.NewMockExecutor("1.2.3.4", append([]ssh.MockCommand{ + ssh.MockCommand{Match: "mkdir -p ", Output: ""}, + }, cmds...)...) + att := releasemeta.MustAttempt("myapp", "abc123") + return mock, &att +} + // TestBuildContainerEnvFiles_SecretsNeverBecomeArgs is the regression test // for the fix in this file: decrypted secrets used to be merged into a map // passed as `docker run -e KEY=value` arguments (visible in `ps aux` / @@ -15,10 +27,10 @@ import ( // must now only ever reach the server via Upload (SFTP-style, never a // shell command string). func TestBuildContainerEnvFiles_SecretsNeverBecomeArgs(t *testing.T) { - mock := ssh.NewMockExecutor("1.2.3.4") + mock, att := attMock() secrets := map[string]string{"API_KEY": "super-secret-value"} - envFiles, err := buildContainerEnvFiles(context.Background(), mock, "myapp", "", nil, nil, secrets) + envFiles, err := buildContainerEnvFiles(context.Background(), mock, "myapp", att, "", nil, nil, secrets) if err != nil { t.Fatalf("buildContainerEnvFiles: %v", err) } @@ -44,9 +56,9 @@ func TestBuildContainerEnvFiles_SecretsNeverBecomeArgs(t *testing.T) { } func TestBuildContainerEnvFiles_PersistedFileComesFirst(t *testing.T) { - mock := ssh.NewMockExecutor("1.2.3.4") + mock, att := attMock() - envFiles, err := buildContainerEnvFiles(context.Background(), mock, "myapp", + envFiles, err := buildContainerEnvFiles(context.Background(), mock, "myapp", att, "/deployments/myapp/.env", map[string]string{"NODE_ENV": "production"}, nil, nil, @@ -63,9 +75,9 @@ func TestBuildContainerEnvFiles_PersistedFileComesFirst(t *testing.T) { } func TestBuildContainerEnvFiles_NoValuesNoUpload(t *testing.T) { - mock := ssh.NewMockExecutor("1.2.3.4") + mock, att := attMock() - envFiles, err := buildContainerEnvFiles(context.Background(), mock, "myapp", "", nil, nil, nil) + envFiles, err := buildContainerEnvFiles(context.Background(), mock, "myapp", att, "", nil, nil, nil) if err != nil { t.Fatalf("buildContainerEnvFiles: %v", err) } @@ -85,11 +97,11 @@ func TestBuildContainerEnvFiles_NoValuesNoUpload(t *testing.T) { // into the SAME file (secrets applied last) rather than splitting them // across -e and --env-file. func TestBuildContainerEnvFiles_SecretWinsOverPlaintextDefault(t *testing.T) { - mock := ssh.NewMockExecutor("1.2.3.4") + mock, att := attMock() appEnv := map[string]string{"API_KEY": "plaintext-default"} secrets := map[string]string{"API_KEY": "real-secret"} - envFiles, err := buildContainerEnvFiles(context.Background(), mock, "myapp", "", appEnv, nil, secrets) + envFiles, err := buildContainerEnvFiles(context.Background(), mock, "myapp", att, "", appEnv, nil, secrets) if err != nil { t.Fatalf("buildContainerEnvFiles: %v", err) } @@ -107,12 +119,12 @@ func TestBuildContainerEnvFiles_SecretWinsOverPlaintextDefault(t *testing.T) { // confusing complaint about a variable name containing whitespace. Catch it at // the source and name the offending variable instead. func TestBuildContainerEnvFiles_RejectsMultilineValues(t *testing.T) { - mock := ssh.NewMockExecutor("1.2.3.4") + mock, att := attMock() appEnv := map[string]string{ "APP_CONFIG": "port: 7880\nrtc:\n tcp_port: 7881\n", } - _, err := buildContainerEnvFiles(context.Background(), mock, "myapp", "", appEnv, nil, nil) + _, err := buildContainerEnvFiles(context.Background(), mock, "myapp", att, "", appEnv, nil, nil) if err == nil { t.Fatal("a multi-line env value must be rejected, not written into the env file") } @@ -121,14 +133,14 @@ func TestBuildContainerEnvFiles_RejectsMultilineValues(t *testing.T) { } // A carriage return alone breaks the format just the same. - _, err = buildContainerEnvFiles(context.Background(), mock, "myapp", "", + _, err = buildContainerEnvFiles(context.Background(), mock, "myapp", att, "", map[string]string{"OTHER": "a\rb"}, nil, nil) if err == nil { t.Fatal("a value containing a carriage return must also be rejected") } // Ordinary single-line values are unaffected. - if _, err := buildContainerEnvFiles(context.Background(), mock, "myapp", "", + if _, err := buildContainerEnvFiles(context.Background(), mock, "myapp", att, "", map[string]string{"FINE": "{a: 1, b: 2}"}, nil, nil); err != nil { t.Fatalf("single-line values must still be accepted: %v", err) } diff --git a/internal/cli/rollback.go b/internal/cli/rollback.go index d3d1150..f88d665 100644 --- a/internal/cli/rollback.go +++ b/internal/cli/rollback.go @@ -75,7 +75,10 @@ func runRollback(flags *Flags, toHash string) error { // Preserve custom TLS termination across rollback. The cert is already // on the server from the last deploy; re-upload to be safe (idempotent). - tlsCert, tlsKey, tlsInternal, err := resolveAppTLS(ctx, executor, appCfg) + // Legacy shared path (nil attempt): the rollback target is not known + // here, and F14-recorded releases override these paths from the record + // with the target's own attempt-scoped cert (F08). + tlsCert, tlsKey, tlsInternal, err := resolveAppTLS(ctx, executor, appCfg, nil) if err != nil { return err } diff --git a/internal/cli/scale.go b/internal/cli/scale.go index edbee00..9d88af4 100644 --- a/internal/cli/scale.go +++ b/internal/cli/scale.go @@ -189,8 +189,10 @@ func rollbackSingleServer(ctx context.Context, appCfg *config.AppConfig, target defer executor.Close() // Preserve custom TLS termination across rollback, same as the - // interactive `teploy rollback` path (internal/cli/rollback.go). - tlsCert, tlsKey, tlsInternal, err := resolveAppTLS(ctx, executor, appCfg) + // interactive `teploy rollback` path (internal/cli/rollback.go) — + // legacy shared paths (nil attempt): the record supplies the target + // release's own attempt-scoped cert when there is one (F08). + tlsCert, tlsKey, tlsInternal, err := resolveAppTLS(ctx, executor, appCfg, nil) if err != nil { return err } @@ -253,7 +255,9 @@ func updateLoadBalancer(ctx context.Context, flags *Flags, appCfg *config.AppCon // Upload + reference the app's custom TLS cert on this LB host so the // load balancer terminates HTTPS the same way the app servers do. - cert, key, internal, tlsErr := resolveAppTLS(ctx, executor, appCfg) + // Legacy shared paths (nil attempt): the LB is not the release's + // target of record — see uploadAppTLS (F08). + cert, key, internal, tlsErr := resolveAppTLS(ctx, executor, appCfg, nil) if tlsErr != nil { executor.Close() return fmt.Errorf("uploading TLS to LB %s: %w", name, tlsErr) diff --git a/internal/cli/singledeploy.go b/internal/cli/singledeploy.go index d91ea29..3900e47 100644 --- a/internal/cli/singledeploy.go +++ b/internal/cli/singledeploy.go @@ -11,6 +11,7 @@ import ( "github.com/useteploy/teploy/internal/config" "github.com/useteploy/teploy/internal/deploy" "github.com/useteploy/teploy/internal/docker" + "github.com/useteploy/teploy/internal/releasemeta" "github.com/useteploy/teploy/internal/secret" "github.com/useteploy/teploy/internal/ssh" ) @@ -63,7 +64,10 @@ func (s *singleServerDeployer) deployApp(ctx context.Context, appCfg *config.App } } - // Build if needed. + // Build if needed. The attempt (F08) keys every artifact this deploy + // generates; minted per server here (each target's artifacts are + // local to that target, like state and pins). + att := releasemeta.MustAttempt(appCfg.App, version) needsBuild := image == "" var buildMode build.Mode if needsBuild { @@ -94,7 +98,10 @@ func (s *singleServerDeployer) deployApp(ctx context.Context, appCfg *config.App return fmt.Errorf("local build: %w", err) } } else { - remoteDir := fmt.Sprintf("/deployments/%s/build", appCfg.App) + // Attempt-scoped build context (F08): a fresh directory per + // (release, attempt), with the previous attempt's build dir as + // rsync's --link-dest basis so transfer stays incremental. + remoteDir := att.BuildDir() if _, err := s.exec.Run(ctx, "mkdir -p "+remoteDir); err != nil { return fmt.Errorf("creating build directory: %w", err) } @@ -108,6 +115,7 @@ func (s *singleServerDeployer) deployApp(ctx context.Context, appCfg *config.App User: s.exec.User(), KeyPath: s.keyPath, Excludes: excludes, + LinkDest: releasemeta.PreviousAttemptBuildDir(ctx, s.exec, appCfg.App, att.ID), }, s.out, s.out); err != nil { return fmt.Errorf("syncing source: %w", err) } @@ -212,8 +220,8 @@ func (s *singleServerDeployer) deployApp(ctx context.Context, appCfg *config.App } } - // Upload custom TLS cert (if configured) before deploy. - tlsCert, tlsKey, tlsInternal, err := resolveAppTLS(ctx, s.exec, appCfg) + // Upload custom TLS cert (if configured) before deploy — attempt-scoped (F08). + tlsCert, tlsKey, tlsInternal, err := resolveAppTLS(ctx, s.exec, appCfg, &att) if err != nil { return err } @@ -243,7 +251,7 @@ func (s *singleServerDeployer) deployApp(ctx context.Context, appCfg *config.App // servers.yml), and decrypted secrets, uploaded to a fresh env file // rather than passed as `docker run -e` args — see // buildContainerEnvFiles for why. - envFiles, err := buildContainerEnvFiles(ctx, s.exec, appCfg.App, envFile, appCfg.Env, tags, deploySecrets) + envFiles, err := buildContainerEnvFiles(ctx, s.exec, appCfg.App, &att, envFile, appCfg.Env, tags, deploySecrets) if err != nil { return err } diff --git a/internal/deploy/deploy.go b/internal/deploy/deploy.go index f668e47..aed8367 100644 --- a/internal/deploy/deploy.go +++ b/internal/deploy/deploy.go @@ -638,6 +638,31 @@ func (d *Deployer) DeployFenced(ctx context.Context, cfg Config, lk *state.Lock) // deploy or backfill. Never abort into abortStateCommit from here. d.recordRelease(ctx, cfg, newState, ports, webBindHost, webContainerName) + // 13c. Prune superseded attempts (F08): attempt directories (build + // contexts, env files, TLS certs) are dead weight once their release + // is outside the rollback window — env is baked into containers at + // create and recreate uses the inspect-derived resolved env, never + // the file. Same protection window as version pruning: current, + // previous, and pinned releases keep their artifacts. + { + var prevHash string + if current != nil { + prevHash = current.CurrentHash + } + protected := []string{cfg.Version, prevHash} + // Pins protect their release's artifacts like versions (F78 + // parity): an unreadable pin file skips the extra protection, and + // that is reported, never silent. + if pins, pinsErr := state.ReadPins(ctx, d.exec, cfg.App); pinsErr == nil { + protected = append(protected, pins...) + } else { + fmt.Fprintf(d.out, "Warning: attempt artifacts protected only as current+previous — pin state could not be read: %v\n", pinsErr) + } + if err := releasemeta.PruneAttempts(ctx, d.exec, cfg.App, protected...); err != nil { + fmt.Fprintf(d.out, "Warning: could not prune superseded attempt artifacts: %v\n", err) + } + } + // 14. Stop the predecessor workload snapshotted in step 6b (all // processes + all replicas). For same-version redeploys the old // containers were renamed to _replaced; remove them after stopping so diff --git a/internal/deploy/fence_test.go b/internal/deploy/fence_test.go index f712163..d0b59a0 100644 --- a/internal/deploy/fence_test.go +++ b/internal/deploy/fence_test.go @@ -108,3 +108,52 @@ func TestDeployFenced_LateHolderRefusedToStartContainers(t *testing.T) { t.Error("late holder must not commit state") } } + +// TestDeployFenced_PrunesSupersededAttempts: after a committed deploy, the +// attempt artifacts of releases outside the protection window are pruned +// (F08) while the deployed release's attempt survives. +func TestDeployFenced_PrunesSupersededAttempts(t *testing.T) { + app := "fency" + mocks := fenceHappyPathMocks(app) + // Attempt prune: the artifact roots list an ancient attempt plus an + // unparsable stray; both roots' listings and the pin read succeed. + mocks = append(mocks, + ssh.MockCommand{Match: "cat /deployments/fency/pins", Output: ""}, + ssh.MockCommand{Match: "ls -1 /deployments/fency/meta/att", Output: "ancient.0000000000000003\nstray"}, + ssh.MockCommand{Match: "ls -1 /deployments/caddy/tls/att", Output: "ancient.0000000000000003"}, + ssh.MockCommand{Match: "rm -rf ", Output: ""}, + ) + mock := ssh.NewMockExecutor("1.2.3.4", mocks...) + lk, err := state.AcquireLockFenced(context.Background(), mock, app) + if err != nil { + t.Fatalf("AcquireLockFenced: %v", err) + } + var buf bytes.Buffer + d := NewDeployer(mock, &buf) + if err := d.DeployFenced(context.Background(), Config{ + App: app, + Domain: "fency.com", + Image: "fency:latest", + Version: "abc123", + Health: HealthConfig{Timeout: 5 * time.Second, Interval: 10 * time.Millisecond}, + }, lk); err != nil { + t.Fatalf("DeployFenced: %v", err) + } + var prunedAncient, prunedStray bool + for _, c := range mock.Calls { + if strings.HasPrefix(c, "rm -rf ") { + if strings.Contains(c, "ancient.") { + prunedAncient = true + } + if strings.Contains(c, "stray") { + prunedStray = true + } + } + } + if !prunedAncient { + t.Error("expected the ancient release's attempt artifacts to be pruned") + } + if prunedStray { + t.Error("an unparsable attempt entry must be kept (fail closed)") + } +} diff --git a/internal/releasemeta/attempt.go b/internal/releasemeta/attempt.go new file mode 100644 index 0000000..4d7a0bd --- /dev/null +++ b/internal/releasemeta/attempt.go @@ -0,0 +1,203 @@ +// Attempt-scoped immutable deploy artifacts (audit F08). +// +// Before F08, the three artifacts a deploy attempt generates were written to +// SHARED per-app paths — the build context at /deployments//build +// (rsynced before the app lock was ever taken), the resolved env file at +// /deployments//.deploy-env, and the TLS cert/key at +// /deployments/caddy/tls/.crt — so two attempts of the same app (a +// racing deploy, a `teploy build` during a deploy) interleaved writes, and +// the F14 record's env-file reference named a path a LATER attempt would +// overwrite. +// +// F08 keys them per release-attempt in the releasemeta namespace: +// +// - build context: /deployments//meta/att/./build +// - env file: /deployments//meta/att/./env +// - TLS cert/key: /deployments/caddy/tls/att/./.{crt,key} +// +// Each attempt gets a fresh random id, so its paths are written exactly once +// and never rewritten by anyone — concurrent attempts cannot interleave +// because they cannot collide. The deploy lease (F16's fenced lock, now +// acquired BEFORE artifact generation on the terminal path) serializes +// attempts on top of that. +// +// TLS deliberately stays under /deployments/caddy/tls rather than moving +// into meta/: the caddy container mounts that directory at /etc/caddy (the +// only mount a server that can do custom TLS provably has), so attempt +// scoping there changes no mount topology. Container-side path: +// /etc/caddy/tls/att/./.crt. +// +// Retention: attempt dirs are dead weight once their release is outside the +// rollback window (env is baked into the container at create; recreate uses +// the inspect-derived resolved env, never the file). PruneAttempts removes +// attempt dirs whose release hash is not protected — the same window +// keep_versions pruning honors — and fails closed on entries it cannot +// parse, like pin pruning (F78). + +package releasemeta + +import ( + "crypto/rand" + "encoding/hex" + "fmt" + "regexp" + "sort" + "strings" + + "context" + + "github.com/useteploy/teploy/internal/ssh" +) + +// attemptIDLen is the random suffix length (16 hex chars). +const attemptIDLen = 16 + +// attemptDirRE matches an attempt directory name: .. The +// hash part reuses validHash's grammar; unparsable names are NEVER pruned. +var attemptDirRE = regexp.MustCompile(`^([A-Za-z0-9_][A-Za-z0-9._-]{0,127})\.([0-9a-f]{16})$`) + +// Attempt identifies one deploy attempt of one release: (app, hash) plus a +// random id that makes every artifact path this attempt writes unique and +// therefore write-once. +type Attempt struct { + App string + Hash string + ID string +} + +// NewAttempt mints an attempt for (app, hash). +func NewAttempt(app, hash string) (Attempt, error) { + if app == "" { + return Attempt{}, fmt.Errorf("attempt requires an app") + } + if !validHash.MatchString(hash) { + return Attempt{}, fmt.Errorf("invalid release id %q for app %q", hash, app) + } + var b [8]byte + if _, err := rand.Read(b[:]); err != nil { + return Attempt{}, fmt.Errorf("generating attempt id: %w", err) + } + return Attempt{App: app, Hash: hash, ID: hex.EncodeToString(b[:])}, nil +} + +// MustAttempt is NewAttempt for call sites that validated app/hash already +// (deploy flows validate the version grammar before this point). +func MustAttempt(app, hash string) Attempt { + a, err := NewAttempt(app, hash) + if err != nil { + panic(fmt.Sprintf("releasemeta: invalid attempt %s@%s: %v", app, hash, err)) + } + return a +} + +// Name is the attempt's directory name under the att/ namespace. +func (a Attempt) Name() string { return a.Hash + "." + a.ID } + +// Dir is the attempt's artifact root: /deployments//meta/att/. +func (a Attempt) Dir() string { + return fmt.Sprintf("%s/%s/meta/att/%s", deploymentsDir, a.App, a.Name()) +} + +// BuildDir is where the attempt's rsync'd build context lives. +func (a Attempt) BuildDir() string { return a.Dir() + "/build" } + +// EnvFile is the attempt's resolved container env file (docker --env-file). +func (a Attempt) EnvFile() string { return a.Dir() + "/env" } + +// TLSDir is the attempt's TLS directory on the HOST. It sits under +// /deployments/caddy/tls (mounted at /etc/caddy in the caddy container) — +// see the package doc for why not under meta/. +func (a Attempt) TLSDir() string { + return fmt.Sprintf("/deployments/caddy/tls/att/%s", a.Name()) +} + +// TLSCertPath / TLSKeyPath are the attempt's cert/key as seen INSIDE the +// caddy container (what the Caddyfile site block references): the host's +// /deployments/caddy/tls/att/... is mounted at /etc/caddy. +func (a Attempt) TLSCertPath() string { return "/etc/caddy/tls/att/" + a.Name() + "/" + a.App + ".crt" } +func (a Attempt) TLSKeyPath() string { return "/etc/caddy/tls/att/" + a.Name() + "/" + a.App + ".key" } + +// tlsAttemptRoot is the host root holding per-attempt TLS directories. +const tlsAttemptRoot = "/deployments/caddy/tls/att" + +// attemptRoot is the host root holding per-attempt artifact directories. +func attemptRoot(app string) string { + return fmt.Sprintf("%s/%s/meta/att", deploymentsDir, app) +} + +// listAttempts lists attempt directory names under root ("" when the +// directory does not exist yet — a first deploy). +func listAttempts(ctx context.Context, exec ssh.Executor, root string) ([]string, error) { + out, err := exec.Run(ctx, "ls -1 "+root+" 2>/dev/null || true") + if err != nil { + return nil, fmt.Errorf("listing attempts under %s: %w", root, err) + } + var names []string + for _, l := range strings.Split(out, "\n") { + if l = strings.TrimSpace(l); l != "" { + names = append(names, l) + } + } + return names, nil +} + +// PruneAttempts removes the attempt directories (artifact root and TLS +// root) of every release hash NOT in keepHashes. Entries whose names do not +// parse as . are kept — an unparsable name is not proof the +// attempt is prunable (F78's rule). Removal failures are returned; callers +// treat pruning as best-effort. +func PruneAttempts(ctx context.Context, exec ssh.Executor, app string, keepHashes ...string) error { + keep := make(map[string]bool, len(keepHashes)) + for _, h := range keepHashes { + if h != "" { + keep[h] = true + } + } + var failures []string + for _, root := range []string{attemptRoot(app), tlsAttemptRoot} { + names, err := listAttempts(ctx, exec, root) + if err != nil { + return err + } + for _, name := range names { + m := attemptDirRE.FindStringSubmatch(name) + if m == nil || keep[m[1]] { + continue + } + if _, rmErr := exec.Run(ctx, "rm -rf "+ssh.ShellQuote(root+"/"+name)); rmErr != nil { + failures = append(failures, root+"/"+name) + } + } + } + if len(failures) > 0 { + return fmt.Errorf("could not remove %d pruned attempt dir(s): %s", len(failures), strings.Join(failures, ", ")) + } + return nil +} + +// PreviousAttemptBuildDir returns the build directory of the most recent +// OTHER attempt (any release hash) — the rsync --link-dest basis, so an +// attempt-scoped build context still transfers incrementally and shares +// unchanged files by hardlink instead of copying them. Empty when there is +// none (first attempt). Best-effort by contract: any failure simply means a +// full transfer. +func PreviousAttemptBuildDir(ctx context.Context, exec ssh.Executor, app, excludeID string) string { + names, err := listAttempts(ctx, exec, attemptRoot(app)) + if err != nil { + return "" + } + filtered := names[:0] + for _, n := range names { + if m := attemptDirRE.FindStringSubmatch(n); m != nil && m[2] != excludeID { + filtered = append(filtered, n) + } + } + if len(filtered) == 0 { + return "" + } + // Deterministic pick: lexicographically greatest name. The id is + // random, so this is not chronology — it does not need to be; any + // recent-ish basis gives rsync its delta. + sort.Strings(filtered) + return attemptRoot(app) + "/" + filtered[len(filtered)-1] + "/build" +} diff --git a/internal/releasemeta/attempt_test.go b/internal/releasemeta/attempt_test.go new file mode 100644 index 0000000..c952b04 --- /dev/null +++ b/internal/releasemeta/attempt_test.go @@ -0,0 +1,121 @@ +package releasemeta + +import ( + "context" + "strings" + "testing" + + "github.com/useteploy/teploy/internal/ssh" +) + +func TestNewAttempt_IDGrammarAndUniqueness(t *testing.T) { + a1, err := NewAttempt("myapp", "abc123") + if err != nil { + t.Fatalf("NewAttempt: %v", err) + } + a2, err := NewAttempt("myapp", "abc123") + if err != nil { + t.Fatalf("NewAttempt: %v", err) + } + if a1.ID == a2.ID { + t.Error("two attempts of the same release must have distinct ids") + } + if got, want := a1.Name(), "abc123."+a1.ID; got != want { + t.Errorf("Name: got %s want %s", got, want) + } + if _, err := NewAttempt("myapp", "../escape"); err == nil { + t.Error("an invalid release id must be rejected") + } +} + +func TestAttemptPaths_AreAttemptScoped(t *testing.T) { + a := MustAttempt("myapp", "abc123") + if !strings.HasPrefix(a.BuildDir(), "/deployments/myapp/meta/att/abc123.") { + t.Errorf("BuildDir not in the attempt namespace: %s", a.BuildDir()) + } + if !strings.HasPrefix(a.EnvFile(), "/deployments/myapp/meta/att/abc123.") { + t.Errorf("EnvFile not in the attempt namespace: %s", a.EnvFile()) + } + if !strings.HasPrefix(a.TLSDir(), "/deployments/caddy/tls/att/abc123.") { + t.Errorf("TLSDir not under the caddy tls att namespace: %s", a.TLSDir()) + } + if want := "/etc/caddy/tls/att/" + a.Name() + "/myapp.crt"; a.TLSCertPath() != want { + t.Errorf("TLSCertPath: got %s want %s", a.TLSCertPath(), want) + } + if want := "/etc/caddy/tls/att/" + a.Name() + "/myapp.key"; a.TLSKeyPath() != want { + t.Errorf("TLSKeyPath: got %s want %s", a.TLSKeyPath(), want) + } + // A second attempt of the same release shares NO path with the first. + b := MustAttempt("myapp", "abc123") + if a.BuildDir() == b.BuildDir() || a.EnvFile() == b.EnvFile() || a.TLSDir() == b.TLSDir() { + t.Error("attempts of the same release must not share artifact paths") + } +} + +func TestPruneAttempts_ProtectsKeepSetAndUnparsable(t *testing.T) { + mock := ssh.NewMockExecutor("1.2.3.4", + // Artifact root listing: a protected current attempt, a protected + // previous attempt, an unprotected old one, and an unparsable name + // that must be KEPT (fail closed, F78 parity). + ssh.MockCommand{Match: "ls -1 /deployments/myapp/meta/att", Output: strings.Join([]string{ + "newhash.0000000000000001", + "oldhash.0000000000000002", + "ancient.0000000000000003", + "stray-directory", + }, "\n")}, + ssh.MockCommand{Match: "ls -1 /deployments/caddy/tls/att", Output: strings.Join([]string{ + "ancient.0000000000000003", + }, "\n")}, + ssh.MockCommand{Match: "rm -rf", Output: ""}, + ) + if err := PruneAttempts(context.Background(), mock, "myapp", "newhash", "oldhash"); err != nil { + t.Fatalf("PruneAttempts: %v", err) + } + var removed []string + for _, c := range mock.Calls { + if strings.HasPrefix(c, "rm -rf ") { + removed = append(removed, c) + } + } + if len(removed) != 2 { + t.Fatalf("expected exactly the ancient attempt's two dirs removed, got %v", removed) + } + for _, r := range removed { + if !strings.Contains(r, "ancient.") { + t.Errorf("removed a protected or unparsable entry: %s", r) + } + } +} + +func TestPruneAttempts_AbsentRootsAreNoops(t *testing.T) { + mock := ssh.NewMockExecutor("1.2.3.4", + ssh.MockCommand{Match: "ls -1", Output: ""}, + ) + if err := PruneAttempts(context.Background(), mock, "myapp", "newhash"); err != nil { + t.Fatalf("PruneAttempts on a fresh install: %v", err) + } + for _, c := range mock.Calls { + if strings.HasPrefix(c, "rm -rf") { + t.Errorf("nothing should be removed, saw %s", c) + } + } +} + +func TestPreviousAttemptBuildDir(t *testing.T) { + mock := ssh.NewMockExecutor("1.2.3.4", + ssh.MockCommand{Match: "ls -1 /deployments/myapp/meta/att", Output: strings.Join([]string{ + "aaa.0000000000000001", + "bbb.0000000000000002", + "not-an-attempt", + }, "\n")}, + ) + got := PreviousAttemptBuildDir(context.Background(), mock, "myapp", "0000000000000002") + if want := "/deployments/myapp/meta/att/aaa.0000000000000001/build"; got != want { + t.Errorf("PreviousAttemptBuildDir: got %s want %s", got, want) + } + // Excluding the other parseable attempt selects the remaining one; + // unparsable names never become a basis. + if got := PreviousAttemptBuildDir(context.Background(), mock, "myapp", "0000000000000001"); got != "/deployments/myapp/meta/att/bbb.0000000000000002/build" { + t.Errorf("unexpected basis when 0001 is excluded: %s", got) + } +} From 25f8118ecf794225ca8e975db8d65961b9136032 Mon Sep 17 00:00:00 2001 From: Tyler <53561637+im-tyler@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:39:36 -0700 Subject: [PATCH 3/5] =?UTF-8?q?feat(caddy):=20F48/F49=20=E2=80=94=20struct?= =?UTF-8?q?ured=20route=20representation,=20adapt-API=20gate,=20parser-bas?= =?UTF-8?q?ed=20adoption?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New vendored structural parser (routes.go): top-level site blocks with verbatim bodies, global-options blocks, snippets, comments, quoted strings, multi-line backtick literals (the maintenance page's own shape), and heredocs. It fails loudly — error, no edit — on unbalanced braces and top-level import (imports resolve against things this parser does not model; guessing is how another host's routes get deleted). F49: foreign-block adoption is decided on the parsed structure. All hosts requested -> whole-block adoption (the old rule, now structural); PARTIAL overlap -> the adopted hosts are removed from the foreign block's address line and the block survives for its remaining hosts with its directives untouched — previously the leftover duplicate site address failed at reload and took the whole edit down with it. Managed (TEPLOY-marked) regions are never adopted. F48: SetMaintenance extracts the current block's tls directive and basic_auth/forward_auth spans (ExtractPolicy) and carries them into the maintenance block — enabling maintenance no longer silently drops HTTPS termination and auth for the duration. An unextractable policy fails the toggle rather than guessing. Adapt-API integration (adapt.go): the HARD pre-write gate runs the SERVER's own binary (docker exec -i caddy caddy adapt, content over stdin) so validation uses the exact caddy that will serve the config. A LOCAL caddy in PATH is the advisory/debug surface and the cross-check oracle for the parser — deliberately not a gate, because version/module drift makes a local binary reject legitimate caddy_extra directives from custom server builds (found live: rate_limit under stock caddy). Tests: parser/adoption/policy behavioral tests; stub-binary tests for the local adapt plumbing (no caddy exists in this environment — stated, not faked); a PATH-gated cross-check against real caddy (v2.10.2 built from source) proving the parser's host extraction agrees with adapt's JSON and that F48/F49 outputs adapt cleanly — skipped where no binary is installed (CI). --- internal/caddy/adapt.go | 84 +++++ internal/caddy/caddy.go | 142 +++++---- internal/caddy/caddy_test.go | 29 +- internal/caddy/routes.go | 491 ++++++++++++++++++++++++++++++ internal/caddy/routes_test.go | 360 ++++++++++++++++++++++ internal/caddy/tcl_round2_test.go | 12 +- internal/deploy/deploy_test.go | 5 +- internal/ssh/mock.go | 9 + 8 files changed, 1047 insertions(+), 85 deletions(-) create mode 100644 internal/caddy/adapt.go create mode 100644 internal/caddy/routes.go create mode 100644 internal/caddy/routes_test.go diff --git a/internal/caddy/adapt.go b/internal/caddy/adapt.go new file mode 100644 index 0000000..8fc3e8e --- /dev/null +++ b/internal/caddy/adapt.go @@ -0,0 +1,84 @@ +// Caddy adapt-API integration (audits F48/F49). +// +// `caddy adapt --config - --adapter caddyfile` turns Caddyfile text into +// Caddy's own JSON config — the canonical structured route representation. +// Two consumers: +// +// - The HARD pre-write gate is the SERVER's binary (Client.adaptCheck: +// `docker exec -i caddy caddy adapt` over stdin) — the authoritative +// validator, since it is the exact binary that will serve the config. +// - The LOCAL binary, when one is in PATH (Adapter below), is an +// advisory/debug surface and the cross-check oracle for the vendored +// parser (routes_test). It is deliberately NOT allowed to refuse an +// edit: its version and module set can differ from the server's, and a +// stock local binary would reject legitimate caddy_extra directives +// from custom server builds (rate_limit & co), breaking deploys that +// work today. Binary drift makes a local hard gate a false-positive +// machine, so the reload — run by the server's own caddy, with +// rollback — remains the final authority, as it already was. +// +// The binary runs LOCALLY (adapt is a pure text transform; the Caddyfile +// content arrives over the executor) and is an optional dependency: without +// it, edits proceed on the vendored parser's guarantees plus the server-side +// gate and reload. + +package caddy + +import ( + "bytes" + "context" + "fmt" + "os/exec" + "strings" + "time" +) + +// Adapter is a resolvable local caddy binary. +type Adapter struct { + Path string +} + +// ResolveAdapter finds a caddy binary in PATH ("" when there is none). +func ResolveAdapter() *Adapter { + if path, err := exec.LookPath("caddy"); err == nil { + return &Adapter{Path: path} + } + return nil +} + +// AdaptCaddyfile adapts Caddyfile text to Caddy's JSON config via stdin, +// returning the adapted bytes. Any non-zero exit is an error carrying +// caddy's own stderr — including the line number it choked on. +func (a *Adapter) AdaptCaddyfile(ctx context.Context, content string) ([]byte, error) { + ctx, cancel := context.WithTimeout(ctx, 10*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, a.Path, "adapt", "--config", "-", "--adapter", "caddyfile") + cmd.Stdin = strings.NewReader(content) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + msg := strings.TrimSpace(stderr.String()) + if msg == "" { + msg = err.Error() + } + return nil, fmt.Errorf("caddy adapt rejected the Caddyfile: %s", msg) + } + out := bytes.TrimSpace(stdout.Bytes()) + if len(out) == 0 { + return nil, fmt.Errorf("caddy adapt produced no output") + } + return out, nil +} + +// validateViaAdapt is the LOCAL advisory check: with a local caddy binary, +// content is adapted and any failure reported. Used by tooling and tests; +// never a deploy gate (see the package doc for the drift rationale). +func validateViaAdapt(ctx context.Context, content string) error { + adapter := ResolveAdapter() + if adapter == nil { + return nil + } + _, err := adapter.AdaptCaddyfile(ctx, content) + return err +} diff --git a/internal/caddy/caddy.go b/internal/caddy/caddy.go index bc5160c..58554cf 100644 --- a/internal/caddy/caddy.go +++ b/internal/caddy/caddy.go @@ -323,7 +323,12 @@ p{color:#666} // SetMaintenance enables maintenance mode: the app's domain returns a 503 // maintenance page. The app's current route block is stashed so it can be -// restored by RemoveMaintenance without a redeploy. +// restored by RemoveMaintenance without a redeploy. The current block's TLS +// directive and access gate are carried into the maintenance block (F48): +// enabling maintenance used to silently drop HTTPS termination and auth — +// the site downgraded to ACME/default and went PUBLIC for the duration. +// The policy is lifted from the PARSED current block; a block that cannot +// be parsed fails the maintenance toggle rather than guessing. func (c *Client) SetMaintenance(ctx context.Context, app, domain string) error { hosts, err := parseDomains(domain) if err != nil { @@ -335,7 +340,13 @@ func (c *Client) SetMaintenance(ctx context.Context, app, domain string) error { return c.mutate(ctx, func(prev string) (string, error) { begin := fmt.Sprintf(markerBeginFmt, app) end := fmt.Sprintf(markerEndFmt, app) + var pol SitePolicy if cur := extractCaddyfileBlock(prev, begin, end); cur != "" { + extracted, err := ExtractPolicy(cur) + if err != nil { + return "", fmt.Errorf("reading %s's TLS/access policy for maintenance (route left unchanged): %w", app, err) + } + pol = extracted stash := fmt.Sprintf(maintStashFmt, app) // Never overwrite an existing stash (TCL-25): a SECOND // maintenance-on extracts the app's CURRENT block — which by @@ -349,7 +360,7 @@ func (c *Client) SetMaintenance(ctx context.Context, app, domain string) error { } } } - return renderUpdated(prev, app, hosts, maintenanceBlock(hosts)) + return renderUpdated(prev, app, hosts, maintenanceBlock(hosts, pol)) }) } @@ -410,6 +421,18 @@ func (c *Client) mutate(ctx context.Context, transform func(prev string) (string return nil } + // Pre-write validation gate (F48/F49): the transformed Caddyfile must + // adapt under the SERVER's own caddy binary (docker exec, stdin) before + // it is written — structured edits that break the file are refused + // HERE, with the authoritative binary, instead of being discovered by + // the reload below. The LOCAL caddy (adapt.go) is deliberately NOT the + // gate: its version/modules can differ from the server's, and refusing + // a legitimate caddy_extra directive (e.g. a custom-build module) on + // binary drift would break deploys that work today. + if err := c.adaptCheck(ctx, updated); err != nil { + return fmt.Errorf("refusing to write a Caddyfile the server's caddy rejects: %w", err) + } + if err := c.writeCaddyfile(ctx, updated); err != nil { return err } @@ -494,6 +517,17 @@ func (c *Client) verifyDelivered(ctx context.Context) error { return nil } +// adaptCheck runs the server's own caddy adapt on the proposed Caddyfile +// (streamed via stdin, never written) — the pre-write validation gate. A +// non-zero exit refuses the edit before anything changes on disk. +func (c *Client) adaptCheck(ctx context.Context, content string) error { + err := c.exec.RunInput(ctx, fmt.Sprintf("docker exec -i %s caddy adapt --config - --adapter caddyfile", caddyContainer), strings.NewReader(content)) + if err != nil { + return fmt.Errorf("caddy adapt: %w", err) + } + return nil +} + // applyManagedBlock upserts (block != "") or removes (block == "") the app's // marker-delimited block, adopting any foreign block for the same hosts. func (c *Client) applyManagedBlock(ctx context.Context, app string, hosts []string, block string) error { @@ -536,7 +570,10 @@ func renderUpdated(prev, app string, hosts []string, block string) (string, erro } } if len(hosts) > 0 { - updated = removeForeignHostBlocks(updated, hosts) + updated, err = adoptForeignBlocks(updated, hosts) + if err != nil { + return "", err + } } if block != "" { @@ -630,65 +667,11 @@ func (c *Client) releaseLock(ctx context.Context) { c.exec.Run(rctx, "rmdir "+lockDir+" 2>/dev/null || true") } -// removeForeignHostBlocks strips top-level Caddyfile site blocks that serve -// ONLY the given hosts and are NOT inside a Teploy marker region. This lets -// a deploy adopt a domain previously served by a hand-written block, leaving -// a single authoritative block per host. A foreign block that ALSO serves a -// host outside the requested set is LEFT ALONE (audit F49): adopting it -// would delete another application's routes, and the resulting duplicate -// site address fails loudly at caddy reload instead of silently dropping -// hosts nobody asked teploy to touch. -func removeForeignHostBlocks(content string, hosts []string) string { - lines := strings.Split(content, "\n") - out := make([]string, 0, len(lines)) - inMarker := false - for i := 0; i < len(lines); { - line := lines[i] - trimmed := strings.TrimSpace(line) - - if strings.HasPrefix(trimmed, "# TEPLOY BEGIN ") { - inMarker = true - out = append(out, line) - i++ - continue - } - if strings.HasPrefix(trimmed, "# TEPLOY END ") { - inMarker = false - out = append(out, line) - i++ - continue - } - - // A top-level site-block opener: column 0, not a comment, not the - // global options block "{", not a snippet "(name) {", ends with "{". - isOpener := !inMarker && len(line) > 0 && !isSpaceByte(line[0]) && - !strings.HasPrefix(trimmed, "#") && !strings.HasPrefix(trimmed, "{") && - !strings.HasPrefix(trimmed, "(") && strings.HasSuffix(trimmed, "{") - - if isOpener { - depth := strings.Count(line, "{") - strings.Count(line, "}") - block := []string{line} - j := i + 1 - for j < len(lines) && depth > 0 { - block = append(block, lines[j]) - depth += strings.Count(lines[j], "{") - strings.Count(lines[j], "}") - j++ - } - addr := strings.TrimSpace(strings.TrimSuffix(trimmed, "{")) - if addressWithinHosts(addr, hosts) { - i = j // drop the foreign block — every host it serves is being adopted - continue - } - out = append(out, block...) - i = j - continue - } - - out = append(out, line) - i++ - } - return strings.Join(out, "\n") -} +// removeForeignHostBlocks' whole-block rule moved to routes.go's +// adoptForeignBlocks (F49): adoption is decided on the PARSED structure, and +// a foreign block sharing only some of the requested hosts has those hosts +// removed from its address line instead of being left behind to fail at +// reload. // addressWithinHosts reports whether EVERY host in a Caddyfile site-address // (e.g. "example.com, www.example.com") is among the given hosts — the @@ -960,17 +943,32 @@ func loadBalancerBlock(hosts []string, upstreams []Upstream, healthPath string, return b.String() } -// maintenanceBlock renders a site block that returns a 503 maintenance page for -// the given hosts. Caddy adapts the `respond` directive to a static_response -// handler, which the dashboard detects as maintenance mode. -func maintenanceBlock(hosts []string) string { - // No custom-TLS param here — maintenance mode doesn't carry TLS config - // through today (a separate, pre-existing gap, not this fix's scope). - // Zero-value TLS still gets the right outcome for THIS fix: a non-public - // host falls back to http:// same as its regular route block would. +// maintenanceBlock renders a site block that returns a 503 maintenance page +// for the given hosts, preserving the site's TLS directive and access gate +// (F48) — the extracted policy is verbatim, so maintenance holds the same +// security envelope the real route did. A TLS directive present means the +// operator explicitly opted into HTTPS for these hosts, so no http:// +// scheme downgrade is applied to non-public addresses (mirrors +// siteAddresses' wantsTLS rule). +func maintenanceBlock(hosts []string, pol SitePolicy) string { + var tlsLine string + if t := strings.TrimSpace(pol.TLS); t != "" { + tlsLine = t + "\n" + } + var accessSpan string + for _, a := range pol.Access { + accessSpan += a + "\n" + } + schemeHosts := hosts + if tlsLine == "" { + // Same fallback as a regular route with no TLS: a non-public host + // gets an explicit http:// scheme so Caddy does not hang on an + // ACME challenge that can never complete. + schemeHosts = siteAddresses(hosts, TLS{}) + } return fmt.Sprintf( - "%s {\n\theader Content-Type \"text/html; charset=utf-8\"\n\theader Retry-After \"3600\"\n\trespond 503 {\n\t\tbody `%s`\n\t}\n}", - strings.Join(siteAddresses(hosts, TLS{}), ", "), maintenancePage, + "%s {\n%s%s\theader Content-Type \"text/html; charset=utf-8\"\n\theader Retry-After \"3600\"\n\trespond 503 {\n\t\tbody `%s`\n\t}\n}", + strings.Join(schemeHosts, ", "), tlsLine, accessSpan, maintenancePage, ) } diff --git a/internal/caddy/caddy_test.go b/internal/caddy/caddy_test.go index 7e87d7b..476bf3d 100644 --- a/internal/caddy/caddy_test.go +++ b/internal/caddy/caddy_test.go @@ -310,23 +310,34 @@ func TestRemoveMaintenance(t *testing.T) { } } -func TestRemoveForeignHostBlocks(t *testing.T) { - // The foreign block serves BOTH drop.com and www.drop.com; adopting it - // is only safe when the deploy takes over every host it serves (F49). +func TestAdoptForeignBlocks(t *testing.T) { + // The foreign block serves BOTH drop.com and www.drop.com; a full + // takeover adopts it wholesale, a partial one now REWRITES its address + // line instead of leaving a duplicate site address to fail at reload + // (F49's structured partial match). in := "{\n\tadmin 127.0.0.1:2019\n}\n\n" + "keep.com {\n\treverse_proxy keep:80\n}\n\n" + "drop.com, www.drop.com {\n\treverse_proxy old:80\n}\n\n" + "# TEPLOY BEGIN protected\ndrop.com {\n\treverse_proxy managed:80\n}\n# TEPLOY END protected\n" - // Partial takeover (drop.com only): the multi-host foreign block must - // SURVIVE — dropping it would delete www.drop.com's route, and the - // duplicate site address then fails loudly at caddy reload instead. - got := removeForeignHostBlocks(in, []string{"drop.com"}) + // Partial takeover (drop.com only): the block SURVIVES for its other + // host, with the adopted host removed from its address line — and its + // directives untouched. + got, err := adoptForeignBlocks(in, []string{"drop.com"}) + if err != nil { + t.Fatalf("adoptForeignBlocks: %v", err) + } if !strings.Contains(got, "reverse_proxy old:80") { t.Errorf("partially-adopted multi-host foreign block was removed:\n%s", got) } + if !strings.Contains(got, "www.drop.com {") || strings.Contains(got, "drop.com, www.drop.com {") { + t.Errorf("the adopted host must be removed from the foreign address line:\n%s", got) + } - got = removeForeignHostBlocks(in, []string{"drop.com", "www.drop.com"}) + got, err = adoptForeignBlocks(in, []string{"drop.com", "www.drop.com"}) + if err != nil { + t.Fatalf("adoptForeignBlocks: %v", err) + } if strings.Contains(got, "reverse_proxy old:80") { t.Errorf("foreign drop.com block not removed:\n%s", got) } @@ -552,7 +563,7 @@ func TestReverseProxyBlock_CustomCertKeepsRealHost(t *testing.T) { } func TestMaintenanceBlock_NonPublicDomainGetsPlainHTTP(t *testing.T) { - got := maintenanceBlock([]string{"192.168.1.114"}) + got := maintenanceBlock([]string{"192.168.1.114"}, SitePolicy{}) if !strings.HasPrefix(got, "http://192.168.1.114 {") { t.Errorf("maintenanceBlock for a bare IP should start with http://, got:\n%s", got) } diff --git a/internal/caddy/routes.go b/internal/caddy/routes.go new file mode 100644 index 0000000..be63cae --- /dev/null +++ b/internal/caddy/routes.go @@ -0,0 +1,491 @@ +// Structured Caddyfile site-block representation (audits F48/F49). +// +// The managed-block machinery above this file works on string fragments: +// adoption of a foreign (hand-written) block is decided by brace counting +// and whole-block replacement, and maintenance mode cannot carry the site's +// TLS/access policy because there is no structure to extract it from. This +// file is the vendored structural parser both of those need — no caddy +// binary required. It is deliberately small: it recognizes top-level site +// blocks (address line + brace-balanced body), the global options block, +// named snippets, comments, quoted strings, and Caddyfile heredocs, and it +// FAILS LOUDLY (returns an error) on anything it cannot represent +// faithfully: unbalanced braces, top-level `import` (imports must be +// resolved against snippets/paths this parser does not model). A parse +// failure aborts the edit before anything is written — the same posture +// the whole-block-only rule had, one notch earlier. +// +// Known limitation, shared with every line-oriented consumer of the +// Caddyfile in this package: braces inside quoted arguments on a directive +// line are counted naively (quotes are recognized, but a brace inside a +// backtick heredoc opener is not). Teploy-rendered blocks never emit that +// shape; a hand-written one that does fails the balance check rather than +// mis-adopting. + +package caddy + +import ( + "fmt" + "strings" +) + +// SiteBlock is one top-level Caddyfile site block, kept verbatim. +type SiteBlock struct { + // Addresses are the raw site-address tokens from the block's address + // line, comma-split and whitespace-trimmed but otherwise untouched + // (scheme, port, path included as written). + Addresses []string + // Lines is the block verbatim: address line, body, closing brace. + Lines []string +} + +// logicalLines merges physical lines while an unpaired backtick literal is +// open — Caddy backtick literals span lines, and the maintenance page body +// is one (it also carries CSS braces that must never be depth-counted). +// Merged entries keep their embedded newlines so re-emission stays +// verbatim. +func logicalLines(lines []string) []string { + var out []string + var buf []string + open := false + flush := func() { + if len(buf) > 0 { + out = append(out, strings.Join(buf, "\n")) + buf = nil + } + } + for _, l := range lines { + if open { + buf = append(buf, l) + if oddBackticks(l) { + open = false + flush() + } + continue + } + if oddBackticks(l) { + buf = []string{l} + open = true + continue + } + out = append(out, l) + } + flush() + return out +} + +// oddBackticks reports whether the line's backtick count is odd (a literal +// opened or closed but not both). Backticks inside single/double quotes are +// rare enough in Caddyfiles that parity is the honest heuristic; a mismatch +// degrades to a merged logical line, never to silently mis-parsed braces. +func oddBackticks(line string) bool { + n := strings.Count(line, "`") + return n%2 == 1 +} + +// ParseSites parses content into its top-level site blocks. Everything that +// is not a site block (global options, snippets, comments, blank lines) is +// skipped — callers only need blocks; re-rendering the whole Caddyfile is +// explicitly NOT a goal (managed edits stay surgical). +func ParseSites(content string) ([]SiteBlock, error) { + lines := logicalLines(strings.Split(content, "\n")) + var blocks []SiteBlock + depth := 0 + var cur *SiteBlock + for i := 0; i < len(lines); i++ { + raw := lines[i] + code, err := codeLine(raw) + if err != nil { + return nil, fmt.Errorf("line %d: %w", i+1, err) + } + // Heredoc bodies are verbatim: skip to the terminator. + if term, ok := heredocTerminator(code); ok { + for i+1 < len(lines) { + i++ + if strings.TrimSpace(lines[i]) == term { + break + } + } + if cur != nil { + cur.Lines = append(cur.Lines, raw) + } + continue + } + trimmed := strings.TrimSpace(code) + net := braceNet(code) + + if depth == 0 { + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + if strings.HasPrefix(trimmed, "import") && (len(trimmed) == len("import") || trimmed[len("import")] == ' ') { + return nil, fmt.Errorf("top-level %q cannot be structurally resolved — refusing to edit a Caddyfile whose site blocks may be defined elsewhere", trimmed) + } + if trimmed == "{" || strings.HasPrefix(trimmed, "(") { + // Global options block or named snippet: not a site + // block. Consume its body. + depth = net + continue + } + if !strings.HasSuffix(trimmed, "{") || net <= 0 { + // A top-level bare directive (no site address, e.g. email + // inside nothing) — Caddy rejects these outside blocks, so + // treat as unparseable rather than silently skipping. + return nil, fmt.Errorf("unrecognized top-level line %q — refusing to edit a Caddyfile that does not parse as site blocks", trimmed) + } + addr := strings.TrimSpace(strings.TrimSuffix(trimmed, "{")) + cur = &SiteBlock{Addresses: splitAddressLine(addr), Lines: []string{raw}} + depth = net + continue + } + + // Inside a block (site, global, or snippet — only site blocks + // accumulate). + if cur != nil { + cur.Lines = append(cur.Lines, raw) + } + depth += net + if depth == 0 { + if cur != nil { + blocks = append(blocks, *cur) + cur = nil + } + } + if depth < 0 { + return nil, fmt.Errorf("unbalanced '}' at line %d", i+1) + } + } + if depth != 0 { + return nil, fmt.Errorf("unbalanced '{' — block never closed") + } + return blocks, nil +} + +// codeLine strips a trailing comment from a line (an unquoted '#'), keeping +// the line's leading whitespace — indentation is part of the verbatim +// block. Quotes are respected so a '#' inside an argument does not start a +// comment. +func codeLine(raw string) (string, error) { + var quote byte + for i := 0; i < len(raw); i++ { + c := raw[i] + switch { + case quote != 0: + if c == quote { + quote = 0 + } + case c == '\'' || c == '"' || c == '`': + quote = c + case c == '\\': + i++ + case c == '#': + return raw[:i], nil + } + } + if quote != 0 { + // An unterminated quote mid-line: the Caddyfile tokenizer would + // fail; so do we. + return "", fmt.Errorf("unterminated quote") + } + return raw, nil +} + +// braceNet counts braces outside quotes on a comment-stripped line. +func braceNet(code string) int { + var quote byte + net := 0 + for i := 0; i < len(code); i++ { + c := code[i] + switch { + case quote != 0: + if c == quote { + quote = 0 + } + case c == '\'' || c == '"' || c == '`': + quote = c + case c == '\\': + i++ + case c == '{': + net++ + case c == '}': + net-- + } + } + return net +} + +// heredocTerminator reports whether a code line ends by opening a Caddyfile +// heredoc (a token starting with <<) and returns its terminator. +func heredocTerminator(code string) (string, bool) { + fields := strings.Fields(code) + if len(fields) == 0 { + return "", false + } + last := fields[len(fields)-1] + if !strings.HasPrefix(last, "<<") || len(last) == 2 { + return "", false + } + return last[2:], true +} + +// splitAddressLine splits a site-address line on commas, keeping each +// address as written. +func splitAddressLine(addr string) []string { + var out []string + for _, a := range strings.Split(addr, ",") { + if a = strings.TrimSpace(a); a != "" { + out = append(out, a) + } + } + return out +} + +// addressHosts strips schemes and ports-nothing-else from raw address +// tokens for host comparison (mirrors addressWithinHosts's normalization). +func addressHosts(addrs []string) []string { + out := make([]string, 0, len(addrs)) + for _, a := range addrs { + a = strings.TrimPrefix(a, "https://") + a = strings.TrimPrefix(a, "http://") + if sp := strings.IndexAny(a, " \t"); sp >= 0 { + a = a[:sp] + } + out = append(out, a) + } + return out +} + +// hostsWithin reports whether every host of the address list is among the +// requested set (the whole-block adoption condition). +func hostsWithin(addrHosts, hosts []string) bool { + seen := 0 + for _, a := range addrHosts { + matched := false + for _, h := range hosts { + if a == h { + matched = true + break + } + } + if !matched { + return false + } + seen++ + } + return seen > 0 +} + +// adoptForeignBlocks is F49's structured adoption: for every NON-managed +// top-level site block whose hosts overlap the requested set — +// +// - all its hosts are requested → the whole block is removed (the +// historical whole-block-only rule, now decided on parsed structure); +// - some of its hosts are requested → the adopted hosts are REMOVED from +// its address line and the block is kept for its remaining hosts +// (previously the block was left alone and the duplicate site address +// failed at reload — loud, but it took the whole edit down with it); +// - the block's own directives are never touched — adoption rehomes +// HOSTS, it does not merge policy. +// +// TEPLOY-managed regions are never adopted (their owner will rewrite them +// itself; deleting another app's managed block here would fight the next +// deploy of that app). An unparseable Caddyfile is an error: the caller +// aborts before writing. +func adoptForeignBlocks(content string, hosts []string) (string, error) { + // Parse view: managed regions blanked (line count preserved), so their + // site blocks neither parse into the candidate set nor match edits. + view := strings.Split(content, "\n") + inManaged := false + for i, l := range view { + t := strings.TrimSpace(l) + if strings.HasPrefix(t, markerBeginPrefix) { + inManaged = true + } else if strings.HasPrefix(t, markerEndPrefix) { + inManaged = false + } + if inManaged && !strings.HasPrefix(t, markerBeginPrefix) { + view[i] = "" + } + } + blocks, err := ParseSites(strings.Join(view, "\n")) + if err != nil { + return "", fmt.Errorf("structuring the Caddyfile for foreign-block adoption: %w", err) + } + type edit struct { + firstLine string // rewritten address line ("" = drop the block) + block SiteBlock + } + var edits []edit + for _, b := range blocks { + ah := addressHosts(b.Addresses) + if !hostsWithin(ah, hosts) { + // Partial overlap? Compute kept addresses. + requested := map[string]bool{} + for _, h := range hosts { + requested[h] = true + } + kept := b.Addresses[:0:0] + overlap := false + for i, raw := range b.Addresses { + if requested[ah[i]] { + overlap = true + continue + } + kept = append(kept, raw) + } + if !overlap { + continue // not our business + } + if len(kept) == 0 { + // Cannot happen (hostsWithin false means ≥1 kept), but the + // invariant is load-bearing: guard it anyway. + edits = append(edits, edit{block: b}) + continue + } + indent := leadingWhitespace(b.Lines[0]) + rewritten := indent + strings.Join(kept, ", ") + " {" + edits = append(edits, edit{firstLine: rewritten, block: b}) + continue + } + edits = append(edits, edit{block: b}) + } + if len(edits) == 0 { + return content, nil + } + + // Apply the edits by walking the original logical lines and matching + // block address lines verbatim (first lines are unique per block by + // construction — the same file cannot carry two identical site + // addresses without failing at reload anyway). + drop := map[string]bool{} // address code line → drop whole block + rewrite := map[string]string{} + for _, e := range edits { + key, err := codeLine(e.block.Lines[0]) + if err != nil { + return "", fmt.Errorf("structuring the Caddyfile: %w", err) + } + if e.firstLine == "" { + drop[key] = true + } else { + rewrite[key] = e.firstLine + } + } + var out []string + skipping := false + depth := 0 + for _, raw := range logicalLines(strings.Split(content, "\n")) { + code, err := codeLine(raw) + if err != nil { + return "", fmt.Errorf("structuring the Caddyfile: %w", err) + } + net := braceNet(code) + if skipping { + depth += net + if depth <= 0 { + skipping = false + } + continue + } + if drop[code] && net > 0 { + depth = net + skipping = true + continue + } + if repl, ok := rewrite[code]; ok { + out = append(out, repl) + continue + } + out = append(out, raw) + } + // Collapse any double blank lines the removals opened up. + joined := strings.Join(out, "\n") + for strings.Contains(joined, "\n\n\n") { + joined = strings.ReplaceAll(joined, "\n\n\n", "\n\n") + } + return joined, nil +} + +func leadingWhitespace(s string) string { + for i := 0; i < len(s); i++ { + if s[i] != ' ' && s[i] != '\t' { + return s[:i] + } + } + return s +} + +// SitePolicy is the edge policy extracted from a site block (F48): the +// verbatim tls directive and access-gate directives, which maintenance mode +// must preserve so enabling it does not silently drop HTTPS or auth. +type SitePolicy struct { + TLS string // verbatim "tls ..." line ("" when none) + Access []string // verbatim basic_auth / forward_auth spans +} + +// ExtractPolicy parses one site block (address line + body + closing brace, +// as extractCaddyfileBlock returns) and lifts its direct tls and access +// directives, with nested spans (basic_auth's user list, forward_auth's +// options) kept verbatim. Anything unparseable is an error — a maintenance +// block built from a misread policy is worse than one built from none. +func ExtractPolicy(block string) (SitePolicy, error) { + var pol SitePolicy + lines := logicalLines(strings.Split(block, "\n")) + if len(lines) == 0 { + return pol, nil + } + depth := braceNet(lines[0]) // the address line's opening brace + if depth <= 0 { + return pol, fmt.Errorf("not a site block: %q", lines[0]) + } + for i := 1; i < len(lines); i++ { + raw := lines[i] + code, err := codeLine(raw) + if err != nil { + return pol, err + } + if term, ok := heredocTerminator(code); ok { + for i+1 < len(lines) { + i++ + if strings.TrimSpace(lines[i]) == term { + break + } + } + continue + } + trimmed := strings.TrimSpace(code) + if depth == 1 && trimmed != "" && !strings.HasPrefix(trimmed, "#") { + switch { + case trimmed == "tls" || strings.HasPrefix(trimmed, "tls "): + if pol.TLS != "" { + return pol, fmt.Errorf("duplicate tls directive in site block") + } + pol.TLS = raw + case trimmed == "basic_auth" || strings.HasPrefix(trimmed, "basic_auth "), + trimmed == "forward_auth" || strings.HasPrefix(trimmed, "forward_auth "): + // The directive may open a nested span; capture it whole. + span := []string{raw} + net := braceNet(code) + j := i + for net > 0 && j+1 < len(lines) { + j++ + span = append(span, lines[j]) + inner, err := codeLine(lines[j]) + if err != nil { + return pol, err + } + net += braceNet(inner) + } + if net > 0 { + return pol, fmt.Errorf("unbalanced %s span", strings.Fields(trimmed)[0]) + } + pol.Access = append(pol.Access, strings.Join(span, "\n")) + i = j + continue + } + } + depth += braceNet(code) + if depth <= 0 { + return pol, nil // closing brace of the site block + } + } + return pol, fmt.Errorf("unbalanced site block — no closing brace") +} diff --git a/internal/caddy/routes_test.go b/internal/caddy/routes_test.go new file mode 100644 index 0000000..3f472b8 --- /dev/null +++ b/internal/caddy/routes_test.go @@ -0,0 +1,360 @@ +package caddy + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "testing" +) + +// --- Vendored parser (F48/F49) --- + +func TestParseSites_Structure(t *testing.T) { + in := "{\n\tadmin 127.0.0.1:2019\n}\n\n" + + "(snippet) {\n\theader X-From Snippet\n}\n\n" + + "# a comment about a.com\n" + + "a.com, www.a.com {\n\treverse_proxy a:80\n}\n\n" + + "b.com {\n\ttls internal\n\trespond \"hi {ok}\" 200\n}\n" + blocks, err := ParseSites(in) + if err != nil { + t.Fatalf("ParseSites: %v", err) + } + if len(blocks) != 2 { + t.Fatalf("expected 2 site blocks (global + snippet skipped), got %d: %+v", len(blocks), blocks) + } + if got := strings.Join(blocks[0].Addresses, ","); got != "a.com,www.a.com" { + t.Errorf("block 0 addresses: %q", got) + } + if got := strings.Join(blocks[1].Addresses, ","); got != "b.com" { + t.Errorf("block 1 addresses: %q", got) + } + // The quoted brace inside the respond body must not corrupt depth. + if !strings.Contains(strings.Join(blocks[1].Lines, "\n"), `respond "hi {ok}" 200`) { + t.Errorf("block 1 body mangled: %v", blocks[1].Lines) + } +} + +func TestParseSites_MultilineBacktickBody(t *testing.T) { + // The maintenance block's own shape: a backtick literal spanning lines + // with CSS braces inside — round-trip parsing must survive it. + blk := maintenanceBlock([]string{"myapp.com"}, SitePolicy{}) + blocks, err := ParseSites("# TEPLOY BEGIN myapp\n" + blk + "\n# TEPLOY END myapp\n") + if err != nil { + t.Fatalf("ParseSites on a maintenance block: %v", err) + } + if len(blocks) != 1 { + t.Fatalf("expected the maintenance site block to parse as one block, got %d", len(blocks)) + } +} + +func TestParseSites_FailLoud(t *testing.T) { + cases := map[string]string{ + "unbalanced open": "a.com {\n\treverse_proxy a:80\n", + "unbalanced close": "a.com {\n}\n}\n", + "top-level import": "import sites/*.caddy\n\na.com {\n\trespond 200\n}\n", + "stray directive": "email me@example.com\n", + } + for name, in := range cases { + if _, err := ParseSites(in); err == nil { + t.Errorf("%s: expected a loud parse failure", name) + } + } +} + +// --- Structured adoption (F49) --- + +func TestAdoptForeignBlocks_PartialKeepsForeignDirectives(t *testing.T) { + in := "old.com, keep.com, https://secure.org {\n\ttls /c.crt /c.key\n\treverse_proxy legacy:80\n}\n" + got, err := adoptForeignBlocks(in, []string{"old.com", "secure.org"}) + if err != nil { + t.Fatalf("adoptForeignBlocks: %v", err) + } + if !strings.Contains(got, "reverse_proxy legacy:80") { + t.Errorf("the foreign block's directives must survive a partial adoption:\n%s", got) + } + if !strings.Contains(got, "tls /c.crt /c.key") { + t.Errorf("the foreign block's tls directive must survive:\n%s", got) + } + if !strings.Contains(got, "keep.com {") { + t.Errorf("the non-adopted host must remain in the address line:\n%s", got) + } + for _, gone := range []string{"old.com", "secure.org"} { + if strings.Contains(got, gone) { + t.Errorf("adopted host %s must be gone from the address line:\n%s", gone, got) + } + } +} + +func TestAdoptForeignBlocks_UnparseableRefusedBeforeWrite(t *testing.T) { + in := "a.com {\n\treverse_proxy a:80\n" // never closed + if _, err := adoptForeignBlocks(in, []string{"a.com"}); err == nil { + t.Fatal("an unparseable Caddyfile must fail the adoption loudly") + } +} + +// --- Policy extraction + maintenance preservation (F48) --- + +func TestExtractPolicy(t *testing.T) { + blk := "myapp.com {\n" + + "\ttls /etc/caddy/tls/att/abc123.0000000000000001/myapp.crt /etc/caddy/tls/att/abc123.0000000000000001/myapp.key\n" + + "\tbasic_auth {\n\t\talice $2a$14$hash\n\t}\n" + + "\tforward_auth authelia:9091 {\n\t\turi /api/authz/forward-auth\n\t\tcopy_headers Remote-User Remote-Groups\n\t}\n" + + "\treverse_proxy myapp-web-1:3000\n" + + "}\n" + pol, err := ExtractPolicy(blk) + if err != nil { + t.Fatalf("ExtractPolicy: %v", err) + } + if !strings.Contains(pol.TLS, "tls /etc/caddy/tls/att/") { + t.Errorf("tls directive not extracted: %q", pol.TLS) + } + if len(pol.Access) != 2 { + t.Fatalf("expected basic_auth + forward_auth spans, got %+v", pol.Access) + } + if !strings.Contains(pol.Access[0], "alice $2a$14$hash") || !strings.Contains(pol.Access[0], "basic_auth {") { + t.Errorf("basic_auth span not verbatim: %q", pol.Access[0]) + } + if !strings.Contains(pol.Access[1], "copy_headers Remote-User Remote-Groups") { + t.Errorf("forward_auth span not verbatim: %q", pol.Access[1]) + } +} + +func TestMaintenanceBlock_PreservesPolicy(t *testing.T) { + pol := SitePolicy{ + TLS: "\ttls internal", + Access: []string{"\tbasic_auth {\n\t\talice $2a$14$hash\n\t}"}, + } + got := maintenanceBlock([]string{"myapp.com"}, pol) + if !strings.Contains(got, "tls internal") { + t.Errorf("maintenance block dropped the TLS directive:\n%s", got) + } + if !strings.Contains(got, "basic_auth") || !strings.Contains(got, "alice $2a$14$hash") { + t.Errorf("maintenance block dropped the access gate:\n%s", got) + } + if strings.HasPrefix(got, "http://myapp.com") { + t.Errorf("a TLS-carrying maintenance block must not downgrade to http://:\n%s", got) + } +} + +// TestSetMaintenance_PreservesPolicy is the F48 end-to-end: enabling +// maintenance on an app with a custom cert and basic auth keeps both — the +// 503 page is served over the SAME TLS with the SAME gate, instead of +// silently downgrading security for the duration. +func TestSetMaintenance_PreservesPolicy(t *testing.T) { + const existing = "# TEPLOY BEGIN myapp\nmyapp.com {\n" + + "\ttls /etc/caddy/tls/att/abc123.0000000000000001/myapp.crt /etc/caddy/tls/att/abc123.0000000000000001/myapp.key\n" + + "\tbasic_auth {\n\t\talice $2a$14$hash\n\t}\n" + + "\treverse_proxy myapp-web-1:3000\n}\n# TEPLOY END myapp\n" + exec := newFakeStatefulExecutor(map[string]string{caddyfilePath: existing}) + client := NewClient(exec) + + if err := client.SetMaintenance(context.Background(), "myapp", "myapp.com"); err != nil { + t.Fatalf("SetMaintenance: %v", err) + } + written := exec.file(caddyfilePath) + if !strings.Contains(written, "respond 503") { + t.Fatalf("maintenance block not rendered:\n%s", written) + } + if !strings.Contains(written, "tls /etc/caddy/tls/att/abc123.0000000000000001/myapp.crt") { + t.Errorf("TLS directive not carried into maintenance (F48):\n%s", written) + } + if !strings.Contains(written, "alice $2a$14$hash") { + t.Errorf("access gate not carried into maintenance (F48):\n%s", written) + } + // The stash still holds the ORIGINAL route for restoration. + stash := fmt.Sprintf(maintStashFmt, "myapp") + if s := exec.file(stash); !strings.Contains(s, "reverse_proxy myapp-web-1:3000") { + t.Errorf("stash did not capture the original route:\n%s", s) + } +} + +// --- Adapt API (F48/F49) --- +// +// No caddy binary exists in this environment (checked at session start), +// so the binary-plumbing tests drive a STUB executable that mimics +// `caddy adapt`'s contract (stdin Caddyfile → stdout JSON, non-zero exit +// with stderr on invalid input). This tests OUR plumbing honestly; the +// real binary's behavior is exercised only when one is installed. + +func writeStubCaddy(t *testing.T, dir string) { + t.Helper() + script := `#!/bin/sh +# stub caddy adapt: rejects a Caddyfile containing BROKEN, adapts the rest +input=$(cat) +case "$input" in + *BROKEN*) + echo 'stub: adapt failed at line 1' >&2 + exit 1 + ;; +esac +printf '{"apps":{"http":{"servers":{}}}}' +` + path := filepath.Join(dir, "caddy") + if err := os.WriteFile(path, []byte(script), 0755); err != nil { + t.Fatal(err) + } +} + +func TestAdapter_ResolveAndAdapt(t *testing.T) { + dir := t.TempDir() + writeStubCaddy(t, dir) + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + + adapter := ResolveAdapter() + if adapter == nil { + t.Fatal("stub caddy not resolved from PATH") + } + out, err := adapter.AdaptCaddyfile(context.Background(), "a.com {\n\trespond 200\n}\n") + if err != nil { + t.Fatalf("AdaptCaddyfile: %v", err) + } + if !strings.Contains(string(out), `"servers"`) { + t.Errorf("adapted JSON not returned: %s", out) + } + if _, err := adapter.AdaptCaddyfile(context.Background(), "a.com {\n\tBROKEN\n}\n"); err == nil { + t.Fatal("an unadaptable Caddyfile must error, carrying caddy's stderr") + } else if !strings.Contains(err.Error(), "stub: adapt failed") { + t.Errorf("error should carry the adapter's stderr, got: %v", err) + } +} + +func TestValidateViaAdapt_GateRefusesBrokenConfig(t *testing.T) { + dir := t.TempDir() + writeStubCaddy(t, dir) + t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) + + if err := validateViaAdapt(context.Background(), "a.com {\n\trespond 200\n}\n"); err != nil { + t.Fatalf("valid config refused: %v", err) + } + err := validateViaAdapt(context.Background(), "a.com {\n\tBROKEN\n}\n") + if err == nil { + t.Fatal("the adapt gate must refuse a broken Caddyfile before it is written") + } +} + +func TestValidateViaAdapt_NoBinaryIsNoop(t *testing.T) { + // PATH with no caddy anywhere: the gate is honestly absent, not fake. + t.Setenv("PATH", t.TempDir()) + if adapter := ResolveAdapter(); adapter != nil { + t.Fatal("expected no adapter in an empty PATH") + } + if err := validateViaAdapt(context.Background(), "anything at all"); err != nil { + t.Fatalf("absent adapter must be a documented no-op, got %v", err) + } +} + +// TestAdaptCrossCheck_WhenCaddyPresent runs ONLY where a real caddy binary +// is in PATH (a developer machine with caddy installed; CI has none and +// skips). It cross-checks the vendored parser's host extraction against +// `caddy adapt`'s own JSON — the F48/F49 structured representation must +// agree with Caddy's, not just with itself — and proves the F48/F49 OUTPUTS +// (maintenance with preserved policy, partial adoption) adapt cleanly under +// the real binary. This session's run used caddy v2.10.2 built from source. +func TestAdaptCrossCheck_WhenCaddyPresent(t *testing.T) { + adapter := ResolveAdapter() + if adapter == nil { + t.Skip("no caddy binary in PATH — vendored-parser unit tests + stub-binary plumbing tests cover the rest") + } + ctx := context.Background() + realHosts := func(content string) map[string]bool { + t.Helper() + out, err := adapter.AdaptCaddyfile(ctx, content) + if err != nil { + t.Fatalf("real adapt failed: %v", err) + } + var cfg struct { + Apps struct { + HTTP struct { + Servers map[string]struct { + Routes []struct { + Match []struct { + Host []string `json:"host"` + } `json:"match"` + } `json:"routes"` + } `json:"servers"` + } `json:"http"` + } `json:"apps"` + } + if err := json.Unmarshal(out, &cfg); err != nil { + t.Fatalf("parsing adapted JSON: %v", err) + } + hosts := map[string]bool{} + for _, srv := range cfg.Apps.HTTP.Servers { + for _, r := range srv.Routes { + for _, m := range r.Match { + for _, h := range m.Host { + hosts[h] = true + } + } + } + } + return hosts + } + fixtures := map[string]string{ + "global+snippet+sites": "{\n\tadmin 127.0.0.1:2019\n}\n\n(s) {\n\theader X 1\n}\n\na.com, www.a.com {\n\treverse_proxy a:80\n}\n\nb.com {\n\ttls internal\n\trespond \"hi {ok}\" 200\n}\n", + "tls+auth": "myapp.com {\n\ttls /c/a.crt /c/a.key\n\tbasic_auth {\n\t\talice $2a$14$abc\n\t}\n\treverse_proxy x:3000\n}\n", + "forward_auth": "myapp.com {\n\tforward_auth auth:9091 {\n\t\turi /api/verify\n\t\tcopy_headers A B\n\t}\n\treverse_proxy x:3000\n}\n", + "comments": "# leading comment\na.com { # trailing\n\trespond 200 # done\n}\n", + "http scheme": "http://192.168.1.5 {\n\trespond 200\n}\n", + "multisite": "a.com {\n\trespond 200\n}\nb.com, c.com {\n\trespond 201\n}\n", + } + for name, fx := range fixtures { + want := realHosts(fx) + blocks, err := ParseSites(fx) + if err != nil { + t.Errorf("%s: parser failed: %v", name, err) + continue + } + got := map[string]bool{} + for _, b := range blocks { + for _, h := range addressHosts(b.Addresses) { + got[h] = true + } + } + if len(got) != len(want) { + t.Errorf("%s: host sets differ: parser=%v adapt=%v", name, got, want) + continue + } + for h := range got { + if !want[h] { + t.Errorf("%s: parser found host %q the real adapter did not: %v", name, h, want) + } + } + } + + // F48/F49 outputs must adapt cleanly under the real binary. + maint := maintenanceBlock([]string{"myapp.com"}, SitePolicy{TLS: "\ttls internal"}) + if _, err := adapter.AdaptCaddyfile(ctx, maint); err != nil { + t.Errorf("maintenance block with preserved tls does not adapt under real caddy: %v\n%s", err, maint) + } + maintAuth := maintenanceBlock([]string{"myapp.com"}, SitePolicy{Access: []string{"\tbasic_auth {\n\t\talice $2a$14$abc\n\t}"}}) + if _, err := adapter.AdaptCaddyfile(ctx, maintAuth); err != nil { + t.Errorf("maintenance block with preserved basic_auth does not adapt under real caddy: %v\n%s", err, maintAuth) + } + adopted, err := adoptForeignBlocks("old.com, keep.com {\n\treverse_proxy legacy:80\n}\n", []string{"old.com"}) + if err != nil { + t.Fatalf("adoptForeignBlocks: %v", err) + } + if _, err := adapter.AdaptCaddyfile(ctx, adopted); err != nil { + t.Errorf("partial-adoption result does not adapt under real caddy: %v\n%s", err, adopted) + } +} + +// TestMutate_AdaptGateRefusesBrokenConfig: the SERVER-side adapt gate must +// refuse a transform whose output the server's own caddy cannot adapt — +// before anything is written to disk. +func TestMutate_AdaptGateRefusesBrokenConfig(t *testing.T) { + exec := newFakeStatefulExecutor(map[string]string{caddyfilePath: "{\n\tadmin 127.0.0.1:2019\n}\n"}) + exec.adaptErr = fmt.Errorf("adapt: unrecognized directive: nonsense") + client := NewClient(exec) + err := client.SetRoute(context.Background(), "myapp", "myapp.com", "myapp-web-1:3000", 3000, TLS{}, "", nil, Firewall{}, Access{}) + if err == nil || !strings.Contains(err.Error(), "refusing to write") { + t.Fatalf("expected the adapt gate to refuse the edit, got: %v", err) + } + if w := exec.file(caddyfilePath); strings.Contains(w, "myapp.com") { + t.Errorf("a refused edit must not change the Caddyfile:\n%s", w) + } +} diff --git a/internal/caddy/tcl_round2_test.go b/internal/caddy/tcl_round2_test.go index 4905a05..657d216 100644 --- a/internal/caddy/tcl_round2_test.go +++ b/internal/caddy/tcl_round2_test.go @@ -16,8 +16,9 @@ import ( // cannot (TCL-59: prefix-response doubles cannot model filesystem // semantics). Only the commands caddy.Client issues are implemented. type fakeStatefulExecutor struct { - mu sync.Mutex - files map[string][]byte + mu sync.Mutex + files map[string][]byte + adaptErr error // when set, the server-side adapt gate refuses } func newFakeStatefulExecutor(initial map[string]string) *fakeStatefulExecutor { @@ -46,6 +47,13 @@ func (f *fakeStatefulExecutor) Run(ctx context.Context, cmd string) (string, err return "", nil case strings.HasPrefix(cmd, "a=$(docker exec caddy md5sum"): return deliveredOK, nil + case strings.HasPrefix(cmd, "docker exec -i caddy caddy adapt"): + // The pre-write adapt gate: the fake has no caddy; it accepts + // unless the test stages a refusal. + if f.adaptErr != nil { + return "", f.adaptErr + } + return "", nil case strings.HasPrefix(cmd, "mkdir "+lockDir), strings.HasPrefix(cmd, "rmdir "+lockDir): return "", nil case cmd == reloadCmd: diff --git a/internal/deploy/deploy_test.go b/internal/deploy/deploy_test.go index d0f65cd..b0b5482 100644 --- a/internal/deploy/deploy_test.go +++ b/internal/deploy/deploy_test.go @@ -893,8 +893,9 @@ func TestDeploy_PostDeployHookFailure(t *testing.T) { ssh.MockCommand{Match: "a=$(docker exec caddy md5sum", Output: "TEPLOY_CADDY_OK"}, ssh.MockCommand{Match: "docker exec caddy caddy reload", Output: ""}, ssh.MockCommand{Match: "rmdir /deployments/caddy/.lock", Output: ""}, - // Post-deploy hook fails. - ssh.MockCommand{Match: "docker exec", Output: "cache clear failed", Err: fmt.Errorf("exit status 1")}, + // Post-deploy hook fails (scoped to the app container so it cannot + // swallow the caddy adapt gate's docker exec). + ssh.MockCommand{Match: "docker exec myapp-web-abc123", Output: "cache clear failed", Err: fmt.Errorf("exit status 1")}, // Log and lock (deploy still succeeds). ssh.MockCommand{Match: "printf %s", Output: ""}, ssh.MockCommand{Match: "rm -rf /deployments/myapp/.lock", Output: ""}, diff --git a/internal/ssh/mock.go b/internal/ssh/mock.go index 34b94d7..9bd5f11 100644 --- a/internal/ssh/mock.go +++ b/internal/ssh/mock.go @@ -81,6 +81,15 @@ func (m *MockExecutor) Run(ctx context.Context, cmd string) (string, error) { m.mu.Unlock() return "", nil } + // The server-side adapt gate (internal/caddy, F48/F49) streams the + // proposed Caddyfile over stdin; the mock cannot run a real caddy, so + // it models "the server's caddy accepted it" — tests that need the + // refusal register an explicit Err command for the same prefix, which + // wins because matching above takes precedence. + if strings.HasPrefix(cmd, "docker exec -i caddy caddy adapt") { + m.mu.Unlock() + return "", nil + } m.mu.Unlock() return "", fmt.Errorf("mock: unexpected command: %s", cmd) } From 4bb6a7920ed3309aaf86a085029ac78b5f7ff723 Mon Sep 17 00:00:00 2001 From: Tyler <53561637+im-tyler@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:42:13 -0700 Subject: [PATCH 4/5] =?UTF-8?q?feat(config,cli):=20F57=20+=20TCL-32=20?= =?UTF-8?q?=E2=80=94=20opt-in=20strict=20env/overlay=20mode=20(--strict-en?= =?UTF-8?q?v)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One persistent flag, default off, two behaviors: - TCL-32: expandEnvTemplates gains a strict mode that fails the deploy listing every unset ${VAR} referenced by teploy.yml's env: — a typo'd variable name currently deploys fine and breaks at runtime as an empty string. Available on the terminal deploy path and the resident autodeploy path (serve --strict-env, or TEPLOY_STRICT_ENV=1 in the already-installed systemd unit — the env var is the config surface that needs no setup changes). - F57: LoadAppWithDestination gains OverlayOptions{Strict}. Under strict, a destination overlay that NAMES a map/list key with an empty value (env: null, env: {}, publish: [], TOML publish = []) explicitly CLEARS the base's field — the one thing the historical non-zero merge could not express, which is what made presence-aware semantics a schema decision rather than a bug. Non-empty overlays merge exactly as before (key-merge for maps, no replace semantics introduced); scalars are not clearable (a zero scalar already has no merge effect and port: 0 is more likely a mistake than a reset). Default behavior is byte-identical: both strict paths are opt-in only, preserving product compat per the register's owner decision. --- internal/cli/accessory.go | 2 +- internal/cli/autodeploy_serve.go | 29 ++++-- internal/cli/build.go | 2 +- internal/cli/deploy.go | 8 +- internal/cli/envfile.go | 31 ++++++- internal/cli/envfile_test.go | 44 +++++++++ internal/cli/kv.go | 2 +- internal/cli/root.go | 8 ++ internal/config/app.go | 87 ++++++++++++++++- internal/config/hardening_test.go | 10 +- internal/config/healthcheck_test.go | 2 +- internal/config/ingress_test.go | 4 +- internal/config/overlay_strict_test.go | 123 +++++++++++++++++++++++++ internal/config/tls_test.go | 2 +- 14 files changed, 328 insertions(+), 26 deletions(-) create mode 100644 internal/config/overlay_strict_test.go diff --git a/internal/cli/accessory.go b/internal/cli/accessory.go index 472779c..d5cd59b 100644 --- a/internal/cli/accessory.go +++ b/internal/cli/accessory.go @@ -28,7 +28,7 @@ var accessoryDestination string // top when -d is supplied. Falls back to plain LoadApp otherwise. func loadAppCfgForAccessory() (*config.AppConfig, error) { if accessoryDestination != "" { - return config.LoadAppWithDestination(".", accessoryDestination) + return config.LoadAppWithDestination(".", accessoryDestination, config.OverlayOptions{}) } return config.LoadApp(".") } diff --git a/internal/cli/autodeploy_serve.go b/internal/cli/autodeploy_serve.go index 631422c..561cdf6 100644 --- a/internal/cli/autodeploy_serve.go +++ b/internal/cli/autodeploy_serve.go @@ -32,9 +32,10 @@ import ( // real deploy at all (see the autodeploy rebuild for what that cost). func newAutoDeployServeCmd() *cobra.Command { var ( - app string - branch string - port int + app string + branch string + port int + strictEnv bool ) cmd := &cobra.Command{ @@ -43,19 +44,26 @@ func newAutoDeployServeCmd() *cobra.Command { Hidden: true, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { - return runAutoDeployServe(app, branch, port) + // The systemd unit cannot easily grow a flag retroactively; + // the environment variable is the config surface for already + // installed units (set TEPLOY_STRICT_ENV=1 in the unit file). + if os.Getenv("TEPLOY_STRICT_ENV") == "1" { + strictEnv = true + } + return runAutoDeployServe(app, branch, port, strictEnv) }, } cmd.Flags().StringVar(&app, "app", "", "app name (required)") cmd.Flags().StringVar(&branch, "branch", "main", "branch to watch for pushes") cmd.Flags().IntVar(&port, "port", 9876, "port to listen on — 0.0.0.0, reachable from Caddy's docker bridge network; every request still requires a valid HMAC signature") + cmd.Flags().BoolVar(&strictEnv, "strict-env", false, "strict env mode: fail the deploy when env: references an unset ${VAR} (also enabled by TEPLOY_STRICT_ENV=1)") cmd.MarkFlagRequired("app") return cmd } -func runAutoDeployServe(app, branch string, port int) error { +func runAutoDeployServe(app, branch string, port int, strictEnv bool) error { if err := config.ValidateName(app); err != nil { return err } @@ -115,7 +123,7 @@ func runAutoDeployServe(app, branch string, port int) error { go func() { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute) defer cancel() - if err := triggerAutoDeploy(ctx, executor, app, branch, buildDir, out, changedFiles, filesKnown); err != nil { + if err := triggerAutoDeploy(ctx, executor, app, branch, buildDir, out, changedFiles, filesKnown, strictEnv); err != nil { logf("deploy failed: %v", err) } else { logf("deploy complete") @@ -290,7 +298,7 @@ func newWebhookHandler(cfg webhookHandlerConfig) http.HandlerFunc { // needs credentials already configured for the server's user, or it's // skipped with a warning), so this can still fail on a server that was // never successfully cloned. -func triggerAutoDeploy(ctx context.Context, executor ssh.Executor, app, branch, buildDir string, out io.Writer, changedFiles []string, filesKnown bool) error { +func triggerAutoDeploy(ctx context.Context, executor ssh.Executor, app, branch, buildDir string, out io.Writer, changedFiles []string, filesKnown, strictEnv bool) error { // The lock's parent must exist before it can be acquired — a server // whose app was never manually deployed has no /deployments/ yet. if err := state.EnsureAppDir(ctx, executor, app); err != nil { @@ -327,8 +335,11 @@ func triggerAutoDeploy(ctx context.Context, executor ssh.Executor, app, branch, // expansion rule manual deploys use (audit F59/F66): without this, the // same manifest received different container env depending on whether // the deploy was manual or webhook-triggered — encrypted-file secrets - // simply went missing on the webhook path. - expandEnvTemplates(appCfg.Env) + // simply went missing on the webhook path. strictEnv (F57/TCL-32) + // makes unset variables fail loudly here too. + if err := expandEnvTemplates(appCfg.Env, strictEnv); err != nil { + return err + } if len(appCfg.EnvFiles) > 0 { fileVars, err := env.LoadLocalEnvFiles(ctx, buildDir, appCfg.EnvFiles) if err != nil { diff --git a/internal/cli/build.go b/internal/cli/build.go index 880a8c7..8719150 100644 --- a/internal/cli/build.go +++ b/internal/cli/build.go @@ -69,7 +69,7 @@ func runBuild(flags *Flags, version, destination string) error { var appCfg *config.AppConfig var err error if destination != "" { - appCfg, err = config.LoadAppWithDestination(".", destination) + appCfg, err = config.LoadAppWithDestination(".", destination, config.OverlayOptions{Strict: flags.StrictEnv}) } else { appCfg, err = config.LoadApp(".") } diff --git a/internal/cli/deploy.go b/internal/cli/deploy.go index e543a21..e287c9d 100644 --- a/internal/cli/deploy.go +++ b/internal/cli/deploy.go @@ -140,7 +140,7 @@ func runDeploy(flags *Flags, serverName, image, version string, skipDNSCheck boo var appCfg *config.AppConfig var err error if destination != "" { - appCfg, err = config.LoadAppWithDestination(".", destination) + appCfg, err = config.LoadAppWithDestination(".", destination, config.OverlayOptions{Strict: flags.StrictEnv}) } else { appCfg, err = config.LoadApp(".") } @@ -181,7 +181,11 @@ func runDeploy(flags *Flags, serverName, image, version string, skipDNSCheck boo // file whose password contains a literal $ used to hand it to // os.Expand at serialization time and silently alter it based on the // operator's environment (audit F59); file/secret values are literal. - expandEnvTemplates(appCfg.Env) + // --strict-env (F57/TCL-32) turns an unset ${VAR} into a listed failure + // instead of a silent empty expansion. + if err := expandEnvTemplates(appCfg.Env, flags.StrictEnv); err != nil { + return err + } if len(appCfg.EnvFiles) > 0 { fileVars, err := env.LoadLocalEnvFiles(ctx, ".", appCfg.EnvFiles) if err != nil { diff --git a/internal/cli/envfile.go b/internal/cli/envfile.go index c262712..1ce9953 100644 --- a/internal/cli/envfile.go +++ b/internal/cli/envfile.go @@ -24,10 +24,37 @@ var validEnvKey = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) // os.Expand again — a decrypted password containing a literal $ used to be // substituted or emptied according to whatever happened to be in the // operator's environment at deploy time (audit F59). -func expandEnvTemplates(env map[string]string) { +// +// strict (the opt-in --strict-env mode, audit F57/TCL-32) fails the deploy +// listing every ${VAR} that is unset, instead of silently expanding it to +// the empty string — a typo'd variable name currently deploys fine and +// breaks at runtime. Default (non-strict) behavior is unchanged for +// compatibility. +func expandEnvTemplates(env map[string]string, strict bool) error { for k, v := range env { - env[k] = os.Expand(v, os.Getenv) + var missing []string + expanded := os.Expand(v, func(name string) string { + if val, ok := os.LookupEnv(name); ok { + return val + } + missing = append(missing, name) + return "" + }) + if strict && len(missing) > 0 { + seen := map[string]bool{} + var uniq []string + for _, m := range missing { + if !seen[m] { + seen[m] = true + uniq = append(uniq, m) + } + } + return fmt.Errorf("strict-env: env.%s references unset variable(s): %s — set %s or deploy without --strict-env", + k, strings.Join(uniq, ", "), strings.Join(uniq, ", ")) + } + env[k] = expanded } + return nil } // buildContainerEnvFiles computes the full container environment — values diff --git a/internal/cli/envfile_test.go b/internal/cli/envfile_test.go index 3d87b98..7392ef8 100644 --- a/internal/cli/envfile_test.go +++ b/internal/cli/envfile_test.go @@ -145,3 +145,47 @@ func TestBuildContainerEnvFiles_RejectsMultilineValues(t *testing.T) { t.Fatalf("single-line values must still be accepted: %v", err) } } + +// TestExpandEnvTemplates_StrictFailsOnUnset is TCL-32's opt-in: --strict-env +// must fail listing every unset ${VAR} instead of silently expanding it to +// the empty string; the default keeps the historical empty expansion. +func TestExpandEnvTemplates_StrictFailsOnUnset(t *testing.T) { + t.Setenv("PRESENT_VAR", "value") + + // Default: unset variables expand to empty (compat preserved). + env := map[string]string{"A": "${PRESENT_VAR}", "B": "${MISSING_VAR}"} + if err := expandEnvTemplates(env, false); err != nil { + t.Fatalf("default expansion must not fail: %v", err) + } + if env["A"] != "value" || env["B"] != "" { + t.Errorf("unexpected default expansion: %+v", env) + } + + // Strict: unset variables fail the deploy, naming key and variables. + env = map[string]string{"A": "${PRESENT_VAR}", "B": "x${MISSING_ONE} y${MISSING_TWO}"} + err := expandEnvTemplates(env, true) + if err == nil { + t.Fatal("strict expansion must fail on unset variables") + } + for _, want := range []string{"env.B", "MISSING_ONE", "MISSING_TWO"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error should name %s, got: %v", want, err) + } + } + if strings.Contains(err.Error(), "PRESENT_VAR") { + t.Errorf("set variables must not be reported: %v", err) + } + // A failing strict expansion must not leave partial mutation. + if env["B"] != "x${MISSING_ONE} y${MISSING_TWO}" { + t.Errorf("failed strict expansion mutated the map: %+v", env) + } + + // Strict with everything set succeeds. + env = map[string]string{"A": "${PRESENT_VAR}"} + if err := expandEnvTemplates(env, true); err != nil { + t.Fatalf("strict expansion with all variables set: %v", err) + } + if env["A"] != "value" { + t.Errorf("strict expansion value: %+v", env) + } +} diff --git a/internal/cli/kv.go b/internal/cli/kv.go index a9f40df..9558cb9 100644 --- a/internal/cli/kv.go +++ b/internal/cli/kv.go @@ -298,7 +298,7 @@ func resolveAppForKv(ctx context.Context, flags *Flags, appName string) (*config var appCfg *config.AppConfig var err error if kvDestination != "" { - appCfg, err = config.LoadAppWithDestination(".", kvDestination) + appCfg, err = config.LoadAppWithDestination(".", kvDestination, config.OverlayOptions{}) } else { appCfg, err = config.LoadApp(".") } diff --git a/internal/cli/root.go b/internal/cli/root.go index b4b9e98..dc740f5 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -14,6 +14,13 @@ type Flags struct { Key string ProjectDir string JSON bool + // StrictEnv is the opt-in strict mode (audits F57/TCL-32): unset + // ${VAR} references in teploy.yml's env: fail the deploy with the + // variable names listed, and a destination overlay that names a + // map/list key with an empty value explicitly CLEARS the base's field + // (`env: {}` means "no env"). Default off: deploys relying on + // empty-expansion keep working. + StrictEnv bool } func NewRootCmd(version string) *cobra.Command { @@ -41,6 +48,7 @@ func NewRootCmd(version string) *cobra.Command { root.PersistentFlags().StringVar(&flags.Key, "key", "", "path to SSH private key") root.PersistentFlags().StringVar(&flags.ProjectDir, "project-dir", "", "run as if teploy was started in this directory") root.PersistentFlags().BoolVar(&flags.JSON, "json", false, "output in JSON format") + root.PersistentFlags().BoolVar(&flags.StrictEnv, "strict-env", false, "strict env/overlay mode: fail on unset ${VAR} in env:, and let an empty map/list in a destination overlay explicitly clear the base value") root.AddCommand(newDeployCmd(flags)) root.AddCommand(newBuildCmd(flags)) diff --git a/internal/config/app.go b/internal/config/app.go index acbcce0..d9cc3d4 100644 --- a/internal/config/app.go +++ b/internal/config/app.go @@ -963,9 +963,22 @@ func LoadApp(dir string) (*AppConfig, error) { return nil, fmt.Errorf("%w in %s", ErrNoConfig, dir) } +// OverlayOptions configures destination-overlay semantics (audit F57's +// opt-in strict mode). +type OverlayOptions struct { + // Strict enables presence-aware overlay semantics: a key PRESENT in + // the overlay file with an empty value (`env:` null, `env: {}`, + // `publish: []`) explicitly CLEARS the base's map/list instead of + // being ignored — the one thing the historical non-zero overlay merge + // could not express. Non-empty values merge exactly as before. The + // default (false) preserves the historical behavior: absent and empty + // are indistinguishable, nothing is ever cleared. + Strict bool +} + // LoadAppWithDestination loads the base config and merges a destination overlay on top. // For example, -d staging loads teploy.yml then merges teploy.staging.yml over it. -func LoadAppWithDestination(dir, dest string) (*AppConfig, error) { +func LoadAppWithDestination(dir, dest string, opts OverlayOptions) (*AppConfig, error) { base, err := LoadApp(dir) if err != nil { return nil, err @@ -985,16 +998,22 @@ func LoadAppWithDestination(dir, dest string) (*AppConfig, error) { } var overlay AppConfig + var present map[string]any if ext == ".toml" { if err := unmarshalAppTOML(data, &overlay); err != nil { return nil, fmt.Errorf("parsing %s: %w", name, err) } + present = tomlTopLevelKeys(data) } else { if err := unmarshalAppYAML(data, &overlay); err != nil { return nil, fmt.Errorf("parsing %s: %w", name, err) } + present = yamlTopLevelKeys(data) } + if opts.Strict { + clearExplicitEmpties(base, &overlay, present) + } mergeConfigs(base, &overlay) if err := base.validate(); err != nil { return nil, fmt.Errorf("invalid config after merging %s: %w", name, err) @@ -1005,6 +1024,72 @@ func LoadAppWithDestination(dir, dest string) (*AppConfig, error) { return nil, fmt.Errorf("destination %q not found — expected teploy.%s.yml or teploy.%s.toml", dest, dest, dest) } +// clearExplicitEmpties implements the strict-mode half of presence-aware +// overlay semantics (F57): for every map/list field the overlay file names +// explicitly with an empty value, reset the base's field so the subsequent +// merge starts from nothing — writing `env: {}` in teploy.prod.yml then +// MEANS "no env", instead of "keep base's env". Scalar fields are not +// clearable (an empty scalar already has no effect under the merge, and a +// zero-value int like port: 0 is more likely a mistake than a reset). +func clearExplicitEmpties(base, overlay *AppConfig, present map[string]any) { + if present == nil { + return + } + if _, ok := present["servers"]; ok && len(overlay.Servers) == 0 { + base.Servers = nil + } + if _, ok := present["env_files"]; ok && len(overlay.EnvFiles) == 0 { + base.EnvFiles = nil + } + if _, ok := present["publish"]; ok && len(overlay.Publish) == 0 { + base.Publish = nil + } + if _, ok := present["volumes"]; ok && len(overlay.Volumes) == 0 { + base.Volumes = nil + } + if _, ok := present["processes"]; ok && len(overlay.Processes) == 0 { + base.Processes = nil + } + if _, ok := present["env"]; ok && len(overlay.Env) == 0 { + base.Env = nil + } + if _, ok := present["healthcheck"]; ok && len(overlay.Healthcheck) == 0 { + base.Healthcheck = nil + } + if _, ok := present["accessories"]; ok && len(overlay.Accessories) == 0 { + base.Accessories = nil + } + if _, ok := present["cache"]; ok && len(overlay.Cache) == 0 { + base.Cache = nil + } + if _, ok := present["headers"]; ok && len(overlay.Headers) == 0 { + base.Headers = nil + } + if _, ok := present["build"]; ok && len(overlay.Build) == 0 { + base.Build = nil + } +} + +// yamlTopLevelKeys decodes just the top-level mapping keys of a YAML +// document (nil for an empty document or a non-mapping one). +func yamlTopLevelKeys(data []byte) map[string]any { + var doc map[string]any + if err := yaml.Unmarshal(data, &doc); err != nil { + return nil + } + return doc +} + +// tomlTopLevelKeys decodes just the top-level table keys of a TOML +// document. +func tomlTopLevelKeys(data []byte) map[string]any { + var doc map[string]any + if err := toml.Unmarshal(data, &doc); err != nil { + return nil + } + return doc +} + // mergeConfigs applies non-zero values from overlay onto base (mutates base). func mergeConfigs(base, overlay *AppConfig) { if overlay.App != "" { diff --git a/internal/config/hardening_test.go b/internal/config/hardening_test.go index 7ce6720..ae8c9a7 100644 --- a/internal/config/hardening_test.go +++ b/internal/config/hardening_test.go @@ -300,7 +300,7 @@ func TestLoadAppWithDestination(t *testing.T) { overlay := "domain: staging.myapp.com\nserver: staging-server\nport: 3001\n" os.WriteFile(filepath.Join(dir, "teploy.staging.yml"), []byte(overlay), 0644) - cfg, err := LoadAppWithDestination(dir, "staging") + cfg, err := LoadAppWithDestination(dir, "staging", OverlayOptions{}) if err != nil { t.Fatalf("LoadAppWithDestination: %v", err) } @@ -330,7 +330,7 @@ func TestLoadAppWithDestination_TOML(t *testing.T) { overlay := "domain = \"staging.myapp.com\"\nserver = \"staging-box\"\n" os.WriteFile(filepath.Join(dir, "teploy.staging.toml"), []byte(overlay), 0644) - cfg, err := LoadAppWithDestination(dir, "staging") + cfg, err := LoadAppWithDestination(dir, "staging", OverlayOptions{}) if err != nil { t.Fatalf("LoadAppWithDestination TOML: %v", err) } @@ -360,7 +360,7 @@ processes: ` os.WriteFile(filepath.Join(dir, "teploy.staging.yml"), []byte(overlay), 0644) - cfg, err := LoadAppWithDestination(dir, "staging") + cfg, err := LoadAppWithDestination(dir, "staging", OverlayOptions{}) if err != nil { t.Fatalf("LoadAppWithDestination: %v", err) } @@ -385,7 +385,7 @@ func TestLoadAppWithDestination_NotFound(t *testing.T) { dir := t.TempDir() os.WriteFile(filepath.Join(dir, "teploy.yml"), []byte("app: myapp\ndomain: myapp.com\n"), 0644) - _, err := LoadAppWithDestination(dir, "production") + _, err := LoadAppWithDestination(dir, "production", OverlayOptions{}) if err == nil { t.Fatal("expected error when destination overlay not found") } @@ -456,7 +456,7 @@ assets: ` os.WriteFile(filepath.Join(dir, "teploy.staging.yml"), []byte(overlay), 0644) - cfg, err := LoadAppWithDestination(dir, "staging") + cfg, err := LoadAppWithDestination(dir, "staging", OverlayOptions{}) if err != nil { t.Fatalf("LoadAppWithDestination: %v", err) } diff --git a/internal/config/healthcheck_test.go b/internal/config/healthcheck_test.go index 0ae11bb..9444200 100644 --- a/internal/config/healthcheck_test.go +++ b/internal/config/healthcheck_test.go @@ -146,7 +146,7 @@ healthcheck: t.Fatal(err) } - cfg, err := LoadAppWithDestination(dir, "staging") + cfg, err := LoadAppWithDestination(dir, "staging", OverlayOptions{}) if err != nil { t.Fatalf("LoadAppWithDestination failed: %v", err) } diff --git a/internal/config/ingress_test.go b/internal/config/ingress_test.go index f995a3d..082bae8 100644 --- a/internal/config/ingress_test.go +++ b/internal/config/ingress_test.go @@ -317,7 +317,7 @@ port: 3000 if err := os.WriteFile(filepath.Join(dir, "teploy.home.yml"), []byte(overlay), 0644); err != nil { t.Fatal(err) } - cfg, err := LoadAppWithDestination(dir, "home") + cfg, err := LoadAppWithDestination(dir, "home", OverlayOptions{}) if err != nil { t.Fatalf("LoadAppWithDestination: %v", err) } @@ -339,7 +339,7 @@ domain: myapp.com if err := os.WriteFile(filepath.Join(dir, "teploy.prod.yml"), []byte(overlay), 0644); err != nil { t.Fatal(err) } - cfg, err := LoadAppWithDestination(dir, "prod") + cfg, err := LoadAppWithDestination(dir, "prod", OverlayOptions{}) if err != nil { t.Fatalf("LoadAppWithDestination: %v", err) } diff --git a/internal/config/overlay_strict_test.go b/internal/config/overlay_strict_test.go new file mode 100644 index 0000000..91c9b87 --- /dev/null +++ b/internal/config/overlay_strict_test.go @@ -0,0 +1,123 @@ +package config + +import ( + "os" + "path/filepath" + "testing" +) + +// TestOverlayStrictExplicitClearing is F57's opt-in semantics: under +// OverlayOptions{Strict: true}, a destination overlay that NAMES a +// map/list key with an empty value clears the base's field; the default +// (non-strict) behavior is untouched for compatibility. +func TestOverlayStrictExplicitClearing(t *testing.T) { + dir := t.TempDir() + write := func(t *testing.T, name, content string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0644); err != nil { + t.Fatal(err) + } + } + write(t, "teploy.yml", `app: myapp +server: prod +domain: myapp.com +env: + KEEP_ME: "yes" + REPLACE_ME: "base" +publish: + - "3001:3001" +volumes: + data: /var/lib/data +`) + write(t, "teploy.staging.yml", `app: myapp +env: + REPLACE_ME: "staging" +publish: [] +volumes: {} +`) + + t.Run("default keeps base (compat)", func(t *testing.T) { + cfg, err := LoadAppWithDestination(dir, "staging", OverlayOptions{}) + if err != nil { + t.Fatalf("LoadAppWithDestination: %v", err) + } + if cfg.Env["KEEP_ME"] != "yes" { + t.Errorf("base env key must survive a default merge, got env=%v", cfg.Env) + } + if cfg.Env["REPLACE_ME"] != "staging" { + t.Errorf("overlay env key must win, got %q", cfg.Env["REPLACE_ME"]) + } + if len(cfg.Publish) != 1 || cfg.Publish[0] != "3001:3001" { + t.Errorf("empty overlay publish must be ignored by default, got %v", cfg.Publish) + } + if len(cfg.Volumes) != 1 { + t.Errorf("empty overlay volumes must be ignored by default, got %v", cfg.Volumes) + } + }) + + t.Run("strict clears explicitly-empty fields", func(t *testing.T) { + cfg, err := LoadAppWithDestination(dir, "staging", OverlayOptions{Strict: true}) + if err != nil { + t.Fatalf("LoadAppWithDestination: %v", err) + } + // env was named with a NON-empty value: key-merge as before. + if cfg.Env["REPLACE_ME"] != "staging" { + t.Errorf("overlay env key must still win, got %q", cfg.Env["REPLACE_ME"]) + } + if cfg.Env["KEEP_ME"] != "yes" { + t.Errorf("non-empty env overlay key-merges (replace semantics not introduced), got env=%v", cfg.Env) + } + // publish/volumes were named EMPTY: cleared. + if len(cfg.Publish) != 0 { + t.Errorf("explicitly-empty publish must clear the base list, got %v", cfg.Publish) + } + if len(cfg.Volumes) != 0 { + t.Errorf("explicitly-empty volumes must clear the base map, got %v", cfg.Volumes) + } + }) +} + +// A null overlay key (`env:` with no value) also clears under strict — +// presence, not the specific empty spelling, is the signal. +func TestOverlayStrictNullKeyClears(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "teploy.yml"), []byte("app: myapp\nserver: prod\ndomain: myapp.com\nenv:\n A: \"1\"\n"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "teploy.prod.yml"), []byte("app: myapp\nenv:\n"), 0644); err != nil { + t.Fatal(err) + } + cfg, err := LoadAppWithDestination(dir, "prod", OverlayOptions{Strict: true}) + if err != nil { + t.Fatalf("LoadAppWithDestination: %v", err) + } + if len(cfg.Env) != 0 { + t.Errorf("a null env: key must clear under strict mode, got %v", cfg.Env) + } + // And must NOT clear in the default mode. + cfg, err = LoadAppWithDestination(dir, "prod", OverlayOptions{}) + if err != nil { + t.Fatalf("LoadAppWithDestination default: %v", err) + } + if cfg.Env["A"] != "1" { + t.Errorf("default mode must keep base env, got %v", cfg.Env) + } +} + +// TOML overlays get the same semantics (`publish = []` clears). +func TestOverlayStrictTOML(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "teploy.yml"), []byte("app: myapp\nserver: prod\ndomain: myapp.com\npublish:\n - \"3001:3001\"\n"), 0644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "teploy.prod.toml"), []byte("app = \"myapp\"\npublish = []\n"), 0644); err != nil { + t.Fatal(err) + } + cfg, err := LoadAppWithDestination(dir, "prod", OverlayOptions{Strict: true}) + if err != nil { + t.Fatalf("LoadAppWithDestination: %v", err) + } + if len(cfg.Publish) != 0 { + t.Errorf("TOML empty publish must clear under strict, got %v", cfg.Publish) + } +} diff --git a/internal/config/tls_test.go b/internal/config/tls_test.go index d66a3d4..f0fe2a1 100644 --- a/internal/config/tls_test.go +++ b/internal/config/tls_test.go @@ -150,7 +150,7 @@ domain: fylun.ai if err := os.WriteFile(filepath.Join(dir, "teploy.prod.yml"), []byte(overlay), 0644); err != nil { t.Fatal(err) } - cfg, err := LoadAppWithDestination(dir, "prod") + cfg, err := LoadAppWithDestination(dir, "prod", OverlayOptions{}) if err != nil { t.Fatalf("LoadAppWithDestination: %v", err) } From 4ecca94c1e0c1212eec36d0102c7e4df2a447162 Mon Sep 17 00:00:00 2001 From: Tyler <53561637+im-tyler@users.noreply.github.com> Date: Fri, 18 Sep 2026 18:43:13 -0700 Subject: [PATCH 5/5] docs(audit): F08/F16/F48/F49/F57 + TCL-04/05/24/32/51 resolved; F04 annotations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Records the 2026-09-18 family: fenced locks with renewal (F16), attempt-scoped immutable artifacts (F08 — whose F04 dependency turned out not to hold), structured Caddy routes with server-side adapt gate (F48/F49), and the opt-in strict-env mode (F57/TCL-32). F04's entry now carries the dependency notes from this session's stay-out: the attempt id is the artifact-side generation token, and DeployFenced's lock handle is where a generation token rides when F04's design pass starts. F45/TCL-39 annotations updated for F48's landing. --- AUDIT_OPEN.md | 147 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 119 insertions(+), 28 deletions(-) diff --git a/AUDIT_OPEN.md b/AUDIT_OPEN.md index 8683f85..c2ef0ba 100644 --- a/AUDIT_OPEN.md +++ b/AUDIT_OPEN.md @@ -13,12 +13,14 @@ almost all of them the same architectural tail pass 6 already carries, now with the round-2 evidence folded in. Open items: the pass-6 deferred tail minus the F14 family (resolved -2026-09-18, bottom section: F13/F14/F20/F21 plus round-2's TCL-09/TCL-13/ -TCL-14 folded into them), the round-2 residual tail itemized in that -section, and the dependent designs F04/F48/F49/F57/F16 that stay deferred -with their annotations. The 2 upstream/owner items are closed (below). The -two upstream items received from teploy-dash's 2026-09-17 pass are closed -below. +2026-09-18) and the F08/F16/F48/F49/F57 family (resolved 2026-09-18, +bottom section: attempt-scoped artifacts, fenced locks, structured Caddy +routes, strict-env — folding in round-2's TCL-04/TCL-05/TCL-24/TCL-32/ +TCL-51), the round-2 residual tail itemized in that section, and the +dependent designs F04 (and its TCL-10/TCL-15 dependents) that stay +deferred with their annotations. The 2 upstream/owner items are closed +(below). The two upstream items received from teploy-dash's 2026-09-17 +pass are closed below. ## Resolved from this register @@ -158,21 +160,29 @@ defect could corrupt data today. bounded by the per-app lock + content dedup). - F43 — Listener scoped to a private address/Unix socket reachable only by the Caddy bridge. -- F45 — Exact managed-route snapshot/restore transaction (needs F48). +- F45 — Exact managed-route snapshot/restore transaction (needs F48's + structured route representation — LANDED 2026-09-18, see the family + section; the transaction design itself remains open and can now be + built on ParseSites/ExtractPolicy). - F47 — Explicit HTTP/TCP/auto probe modes (compat fallback is deliberate and documented). -- F48 — Maintenance mode preserving TLS/access layers (needs a structured - route representation, i.e. Caddy's adapt API — string fragments cannot - reconstruct policy faithfully). -- F49 — Parser-based foreign-block adoption plans (whole-block-only rule - landed; partial matches now fail loudly at reload instead of deleting - another host's routes). +- F48 — RESOLVED 2026-09-18 (see the F16/F08/F48/F49/F57 family section + at the bottom): maintenance preserves the site's TLS directive and + access gate, extracted from the parsed current block; plus a pre-write + adapt gate run by the SERVER's own caddy binary. +- F49 — RESOLVED 2026-09-18 (see the F16/F08/F48/F49/F57 family section + at the bottom): parser-based foreign-block adoption with structured + partial matches (the foreign block keeps its remaining hosts and + directives); unparseable Caddyfiles abort the edit pre-write. - F50 — Splitting the public static tree from /deployments (filesystem layout migration across live servers; deliberate ops project). - F56 — Explicit pull policy (the warned local fallback is a deliberate out-of-band image story; changing it silently breaks offline deploys). -- F57 — Presence-aware overlay semantics (explicit clearing of - maps/lists, null handling) — schema design decision. +- F57 — RESOLVED 2026-09-18 as an opt-in (see the F16/F08/F48/F49/F57 + family section at the bottom): --strict-env makes an explicitly-empty + map/list in a destination overlay CLEAR the base field. The default + stays presence-blind by the register's compat decision; promoting + strict to the default (if ever) is the remaining owner decision. - F60 — Protected full execution spec per release (public redacted snapshot keeps its current role). Largely delivered 2026-09-18 by F14's per-release record (full execution spec, 0600, server-side, embedded @@ -299,9 +309,11 @@ into each rather than duplicated as new work items. name dedup (F03), publish+replicas rejected at validation. F21 RESOLVED 2026-09-18 (F14-family section); the F04 remainder is unblocked by F14, design remains. -- TCL-04 — F08 (attempt-scoped immutable artifacts under one lease). -- TCL-05 — F16 (fencing/renewal). Containment added this round: the caddy - lock release is detached/bounded so cancellation cannot strand it. +- TCL-04 — RESOLVED 2026-09-18 with F08 (family section at the bottom). +- TCL-05 — RESOLVED 2026-09-18 with F16 (family section at the bottom); + the caddy-lock containment this round added is subsumed by the fenced + app lock (the caddy file lock stays short-lived and unfenced by + design). - TCL-08 — F05/F35 tail (durable journal). Contained pieces landed this round: cleanup failure reporting, caddy verify-failure compensation. - TCL-09 — RESOLVED 2026-09-18: F14/F13 landed (immutable per-release @@ -325,15 +337,15 @@ into each rather than duplicated as new work items. per release), design remains. - TCL-17 — F47 tail (explicit HTTP/TCP/auto probe modes; the 404/3xx TCP fallback is documented deliberate compat). -- TCL-24 — F49 tail (foreign-block adoption by brace counting; parser/ - adapt-API based adoption is the fix). +- TCL-24 — RESOLVED 2026-09-18 with F49 (family section at the bottom): + adoption is parser-based; brace counting is gone. - TCL-28 — F50 (split the public static tree from /deployments). - TCL-31 — env-encoder unification across accessory/seal writers. The app env writer validates records (F73); accessory credential values are generated (no newlines possible) — contained follow-up, registered. -- TCL-32 — strict ${VAR} resolution (fail on unset). Product decision: - would break deploys that currently rely on empty expansion; needs an - explicit opt-in syntax. Owner decision. +- TCL-32 — RESOLVED 2026-09-18 (family section at the bottom): the opt-in + landed as --strict-env / serve --strict-env / TEPLOY_STRICT_ENV=1; + default behavior unchanged per this entry's product decision. - TCL-33 — F24 (resumable OpenBao Setup; mandatory persistence landed). - TCL-34 — OpenBao agent readiness gate + token-sink isolation (new lifecycle surface; shares F24's step-journal design). @@ -342,10 +354,11 @@ into each rather than duplicated as new work items. - TCL-37 — F05/F37 tail (readiness-gated accessory upgrade with verified recovery). - TCL-39 — static route-policy restore needs F13/F48; F13 landed 2026-09-18 - (recorded serving config restored), F48 remains (structured route - representation for policy-layer preservation). Renderer input hardening - (header-name grammar, fallback charset) registered as the contained - follow-up inside that item. + (recorded serving config restored) and F48 landed 2026-09-18 (structured + route representation — ExtractPolicy — is now available for the + policy-layer preservation). The orchestrated restore design and the + renderer input hardening (header-name grammar, fallback charset) remain + the open follow-ups inside this item. - TCL-40 — restore under the app lock + writer quiescence (F37-adjacent; the orchestrated quiesce/cutover boundary is new lifecycle surface). - TCL-41 — F37 (engine-specific consistency contract; verify-backup @@ -357,7 +370,8 @@ into each rather than duplicated as new work items. - TCL-48 — F42 (durable webhook queue). - TCL-49 — F40 (pin webhook builds to the event's commit). - TCL-50 — F60 (complete-plan fingerprint vs display digest). -- TCL-51 — F57 (presence-aware overlay semantics). +- TCL-51 — RESOLVED 2026-09-18 with F57's opt-in (family section at the + bottom). - TCL-54 — platform parity per builder (nixpacks --platform), DetectAt stat distinction, pinned installer. Medium; registered with F63's supply-chain work. @@ -430,3 +444,80 @@ deferred as before — F14 does not unblock them. Gates at the closing commits: `go vet ./...` clean; `go test ./... -race` all packages ok. No push performed. + +## F16 / F08 / F48 / F49 / F57 family (2026-09-18) — resolved + +Four commits closing the architecture items the F14 keying surface +unblocked, plus the strict-env owner decision. Gates at the closing +commits: `go vet ./...` clean; `go test ./... -race` all packages ok. No +push performed. + +- **F16 + TCL-05** (`3381947`) — `internal/state/lock.go`: every auto + lock carries a unique owner token (the fencing token) and is renewed in + the background every staleLockTTL/3; staleness is measured from the + last renewal, so a live-but-slow deploy is never falsely broken — the + stranding hazard the register warned about — while a dead holder still + self-heals after the historical 30-minute window. Effect sites verify + the fence before every effectful phase (deploy/rollback/static), and + the atomic state commit (`state.WriteFenced`) renames under the guard: + a broken holder's late write is refused with ErrFenceLost, never + applied. Recovery paths (restoring displaced containers, route + rollback, cleanup of one's own partial effects) are deliberately + UNFENCED — refusing to clean up is how a fencing design strands an app + mid-incident. The owner token doubles as the fencing token; a separate + monotonic counter adds nothing in this topology (the .lock dir on the + target is the single authority — refusal is exactly "does it still + name us"). MockExecutor evaluates the guard against its recorded file + state, so fence tests prove a refused effect never executes. +- **F08 + TCL-04** (`fd93d7a`) — `internal/releasemeta/attempt.go`: + every deploy attempt mints (app, hash, random id) and writes its + artifacts immutable in the releasemeta namespace — build context at + meta/att/./build (rsync --link-dest against the previous + attempt restores incremental transfer and hardlink-shares unchanged + files), env file at meta/att/./env (the F14 record's + EnvFiles now names bytes no later attempt can overwrite), TLS at + /deployments/caddy/tls/att/./ (kept under the caddy tls dir + — the one mount every custom-TLS server provably has; container path + /etc/caddy/tls/att/…). The terminal deploy path acquires the fenced + lease BEFORE artifact generation, so attempts serialize at the source; + `teploy build` (lockless by design) builds into its own attempt dir + and can no longer interleave with a deploy's rsync. PruneAttempts + protects current + previous + pinned releases and fails closed on + unparsable entries (F78 parity). Rollback/LB TLS uploads keep the + legacy shared paths (nil attempt): pre-F14 records still reference + them and recorded releases override from the record. +- **F48 + F49 + TCL-24** (`25f8118`) — `internal/caddy/routes.go` + + `adapt.go`: a vendored structural Caddyfile parser (site blocks with + verbatim bodies, global options, snippets, comments, quoted strings, + multi-line backtick literals, heredocs; loud errors on unbalanced + braces and top-level import). F49: foreign-block adoption is decided + on the parsed structure — whole-block when all hosts are adopted, + STRUCTURED PARTIAL when not (the foreign block keeps its remaining + hosts and its directives; the duplicate-site-address reload failure is + gone), managed regions never adopted, unparseable files abort the edit + pre-write. F48: SetMaintenance extracts the current block's tls + directive and basic_auth/forward_auth spans (ExtractPolicy) and + carries them into the maintenance block — no more silent TLS/auth + downgrade for the duration. The hard pre-write adapt gate runs the + SERVER's binary (docker exec -i caddy caddy adapt over stdin); a LOCAL + caddy is advisory only (version/module drift makes a local hard gate a + false-positive machine — found live with rate_limit under stock + caddy). Cross-checked against real caddy v2.10.2: parser host + extraction agrees with adapt's JSON on every fixture class, and the + F48/F49 outputs adapt cleanly (PATH-gated test; CI has no binary and + skips — the stub-binary tests cover the local-adapt plumbing). +- **F57 + TCL-32 + TCL-51** (`4bb6a79`) — one opt-in flag, default off: + `--strict-env` (persistent) fails the deploy listing every unset + ${VAR} in teploy.yml's env: (terminal path, autodeploy serve flag, or + TEPLOY_STRICT_ENV=1 for already-installed units) and makes an + explicitly-empty map/list in a destination overlay CLEAR the base + field (`env: {}` / `publish: []` / TOML `publish = []`). Default + behavior byte-identical — the compat decision the register recorded. + +F04 dependency annotations (stay-out honored; recorded for its design +pass): F08's attempt ids provide the artifact-side generation token F04 +wanted, and F08's early-lease restructure (`deployAppConfig` acquiring +the fence before artifact generation) plus `DeployFenced`'s lock-handle +parameter are the seam a generation handoff grows from. F45 can now +build on ParseSites/ExtractPolicy. TCL-15's port allocation remains +independent (the F14 record carries the resolved allocation).