fix(CTRL-T2-1,H10): crash-safe deploy state + fail-closed secret encryption
deploy.go, one slice (both edit SaveAppConfig / the deploy goroutine): CTRL-T2-1 (ghost-deployed on crash): DeployStack wrote app.yaml Deployed:true to disk BEFORE the async 'docker compose up -d'; a crash during the image-pull window left a ghost-deployed stack (Deployed:true, no containers) that DeployStack then refused to redeploy. Now the env is persisted with Deployed:false (transitional), and Deployed:true is written by runComposeDeploy ONLY after up -d succeeds. In-memory Deployed stays true during the pull to preserve the no-stale-Telepítés-button UX. On a post-success save failure, revert so the stack is redeployable. H10 (plaintext secret on encrypt failure): SaveAppConfig logged a WARN then fell through to persist the secret in PLAINTEXT. Now fail-closed: return an error on crypto.Encrypt failure, never write plaintext. Callers already propagate it. Regression tests: H10 fail-closed (+ good-key encrypts) and the CTRL-T2-1 transitional durable-state contract (transitional reads not-deployed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -291,15 +291,24 @@ func (m *Manager) DeployStack(req DeployRequest) (string, error) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Save app.yaml
|
// Save app.yaml.
|
||||||
|
// CTRL-T2-1: persist the env now, but mark the ON-DISK state Deployed:false
|
||||||
|
// until `docker compose up -d` actually succeeds (done in runComposeDeploy).
|
||||||
|
// A crash/power-loss during the image-pull window must NOT leave a
|
||||||
|
// ghost-deployed stack on disk (Deployed:true with no containers), which
|
||||||
|
// DeployStack would then refuse to redeploy. The IN-MEMORY Deployed flag is
|
||||||
|
// still set true below to preserve the "no stale Telepítés button during
|
||||||
|
// pull" UX; only the durable record waits for success.
|
||||||
appCfg := &AppConfig{
|
appCfg := &AppConfig{
|
||||||
Deployed: true,
|
Deployed: true, // in-memory truth (see below); the disk write overrides to false
|
||||||
DeployedAt: time.Now().UTC().Format(time.RFC3339),
|
DeployedAt: time.Now().UTC().Format(time.RFC3339),
|
||||||
Env: env,
|
Env: env,
|
||||||
LockedFields: lockedFields,
|
LockedFields: lockedFields,
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := SaveAppConfig(stackDir, appCfg, m.encKey, SensitiveEnvVars(&meta)); err != nil {
|
diskCfg := *appCfg
|
||||||
|
diskCfg.Deployed = false // transitional: env saved, not yet marked deployed
|
||||||
|
if err := SaveAppConfig(stackDir, &diskCfg, m.encKey, SensitiveEnvVars(&meta)); err != nil {
|
||||||
clearDeploying()
|
clearDeploying()
|
||||||
return "", fmt.Errorf("saving app config: %w", err)
|
return "", fmt.Errorf("saving app config: %w", err)
|
||||||
}
|
}
|
||||||
@@ -359,6 +368,25 @@ func (m *Manager) runComposeDeploy(name, stackDir string, env map[string]string,
|
|||||||
|
|
||||||
m.logger.Printf("[INFO] [stacks] Stack %s deployed successfully (took %.1fs)", name, time.Since(start).Seconds())
|
m.logger.Printf("[INFO] [stacks] Stack %s deployed successfully (took %.1fs)", name, time.Since(start).Seconds())
|
||||||
|
|
||||||
|
// CTRL-T2-1: compose up -d succeeded — only NOW mark deployed on disk.
|
||||||
|
// (DeployStack wrote the env with Deployed:false; flip it true here so the
|
||||||
|
// durable record matches reality and survives a restart.)
|
||||||
|
meta := LoadMetadata(stackDir)
|
||||||
|
if err := SaveAppConfig(stackDir, appCfg, m.encKey, SensitiveEnvVars(&meta)); err != nil {
|
||||||
|
// Running but not durably recorded as deployed. Revert so the customer
|
||||||
|
// can cleanly redeploy rather than be stuck with a half-recorded stack.
|
||||||
|
m.logger.Printf("[ERROR] [stacks] Stack %s: compose succeeded but persisting deployed state failed: %v — reverting", name, err)
|
||||||
|
m.mu.Lock()
|
||||||
|
if s, ok := m.stacks[name]; ok {
|
||||||
|
s.Deployed = false
|
||||||
|
s.Deploying = false
|
||||||
|
s.DeployError = "deploy succeeded but state could not be saved: " + err.Error()
|
||||||
|
s.AppConfig = nil
|
||||||
|
}
|
||||||
|
m.mu.Unlock()
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
// Clear deploying flag
|
// Clear deploying flag
|
||||||
m.mu.Lock()
|
m.mu.Lock()
|
||||||
if s, ok := m.stacks[name]; ok {
|
if s, ok := m.stacks[name]; ok {
|
||||||
@@ -653,14 +681,17 @@ func SaveAppConfig(stackDir string, cfg *AppConfig, encKey []byte, sensitiveVars
|
|||||||
}
|
}
|
||||||
for k, v := range cfg.Env {
|
for k, v := range cfg.Env {
|
||||||
if encKey != nil && sensitiveSet[k] && !crypto.IsEncrypted(v) && v != "" {
|
if encKey != nil && sensitiveSet[k] && !crypto.IsEncrypted(v) && v != "" {
|
||||||
if enc, err := crypto.Encrypt(encKey, v); err == nil {
|
enc, err := crypto.Encrypt(encKey, v)
|
||||||
|
if err != nil {
|
||||||
|
// H10 (fail-closed): NEVER persist a sensitive value in plaintext.
|
||||||
|
// Earlier code logged a WARN and fell through to a plaintext write;
|
||||||
|
// that leaked the secret to disk. Abort the save instead — callers
|
||||||
|
// already propagate this error and the deploy fails cleanly.
|
||||||
|
return fmt.Errorf("encrypting sensitive env var %q (refusing to persist plaintext): %w", k, err)
|
||||||
|
}
|
||||||
saveCfg.Env[k] = enc
|
saveCfg.Env[k] = enc
|
||||||
encryptedCount++
|
encryptedCount++
|
||||||
continue
|
continue
|
||||||
} else {
|
|
||||||
// H10 fix: log encryption failure — value will be saved in plaintext.
|
|
||||||
log.Printf("[WARN] [stacks] Failed to encrypt env var %q: %v — saving as plaintext", k, err)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
saveCfg.Env[k] = v
|
saveCfg.Env[k] = v
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,101 @@
|
|||||||
|
package stacks
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Regression tests for the deploy-lifecycle slice (audit/2026-06-13):
|
||||||
|
// - H10: SaveAppConfig must FAIL CLOSED on an encryption error (never persist
|
||||||
|
// a sensitive value in plaintext).
|
||||||
|
// - CTRL-T2-1: the durable record must read as NOT deployed until a deploy
|
||||||
|
// actually completes, so a crash during the image-pull window leaves a
|
||||||
|
// redeployable stack rather than a ghost-deployed one.
|
||||||
|
|
||||||
|
// TestSaveAppConfigFailsClosedOnEncryptError — H10. Originated as a failing
|
||||||
|
// reconcile test; now a permanent guard. A bad-length encKey makes crypto.Encrypt
|
||||||
|
// fail; SaveAppConfig must return an error and write NO plaintext secret.
|
||||||
|
func TestSaveAppConfigFailsClosedOnEncryptError(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
const secret = "supersecret-pw-do-not-leak"
|
||||||
|
cfg := &AppConfig{Deployed: true, Env: map[string]string{"DB_PASSWORD": secret}}
|
||||||
|
|
||||||
|
badKey := []byte("short") // non-nil, invalid AES key length → crypto.Encrypt errors
|
||||||
|
|
||||||
|
err := SaveAppConfig(dir, cfg, badKey, []string{"DB_PASSWORD"})
|
||||||
|
if err == nil {
|
||||||
|
t.Fatalf("H10: SaveAppConfig returned nil on an encrypt failure — expected a fail-closed error")
|
||||||
|
}
|
||||||
|
|
||||||
|
// No app.yaml should have been written; even if one was, it must not contain
|
||||||
|
// the plaintext secret.
|
||||||
|
if data, readErr := os.ReadFile(filepath.Join(dir, "app.yaml")); readErr == nil {
|
||||||
|
if strings.Contains(string(data), secret) {
|
||||||
|
t.Fatalf("H10: app.yaml contains the secret in plaintext after an encrypt failure:\n%s", data)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSaveAppConfigEncryptsWithGoodKey — companion: a valid key encrypts the
|
||||||
|
// sensitive var (it must not appear in plaintext) and the save succeeds.
|
||||||
|
func TestSaveAppConfigEncryptsWithGoodKey(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
const secret = "supersecret-pw"
|
||||||
|
key := make([]byte, 32) // valid AES-256 key
|
||||||
|
cfg := &AppConfig{Deployed: true, Env: map[string]string{"DB_PASSWORD": secret}}
|
||||||
|
|
||||||
|
if err := SaveAppConfig(dir, cfg, key, []string{"DB_PASSWORD"}); err != nil {
|
||||||
|
t.Fatalf("SaveAppConfig with a valid key failed: %v", err)
|
||||||
|
}
|
||||||
|
data, err := os.ReadFile(filepath.Join(dir, "app.yaml"))
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("reading app.yaml: %v", err)
|
||||||
|
}
|
||||||
|
if strings.Contains(string(data), secret) {
|
||||||
|
t.Fatalf("sensitive value was written in plaintext despite a valid key:\n%s", data)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestTransitionalDeployStateReadsNotDeployed — CTRL-T2-1. DeployStack now
|
||||||
|
// writes the env with Deployed:false before launching compose, and flips it to
|
||||||
|
// Deployed:true only after `up -d` succeeds. This locks the durable-state
|
||||||
|
// semantics ScanStacks relies on: a stack persisted in the transitional state
|
||||||
|
// (the on-disk state left by a crash mid-pull) MUST read as not-deployed, so the
|
||||||
|
// customer can redeploy. The full crash-window is integration-level (needs a
|
||||||
|
// real compose); see the audit's manual repro.
|
||||||
|
func TestTransitionalDeployStateReadsNotDeployed(t *testing.T) {
|
||||||
|
key := make([]byte, 32)
|
||||||
|
sensitive := []string{"DB_PASSWORD"}
|
||||||
|
|
||||||
|
// Transitional state DeployStack writes BEFORE compose succeeds.
|
||||||
|
transitional := t.TempDir()
|
||||||
|
if err := SaveAppConfig(transitional, &AppConfig{
|
||||||
|
Deployed: false, DeployedAt: "2026-06-13T00:00:00Z",
|
||||||
|
Env: map[string]string{"DB_PASSWORD": "x"},
|
||||||
|
}, key, sensitive); err != nil {
|
||||||
|
t.Fatalf("saving transitional config: %v", err)
|
||||||
|
}
|
||||||
|
got := LoadAppConfig(transitional)
|
||||||
|
if got == nil {
|
||||||
|
t.Fatal("LoadAppConfig returned nil for the transitional state")
|
||||||
|
}
|
||||||
|
// This is exactly the expression ScanStacks uses: deployed := cfg != nil && cfg.Deployed
|
||||||
|
if got.Deployed {
|
||||||
|
t.Fatalf("CTRL-T2-1: a deploy that did not complete reads as Deployed=true — ghost-deployed; "+
|
||||||
|
"DeployStack must persist Deployed:false until compose succeeds")
|
||||||
|
}
|
||||||
|
|
||||||
|
// Completed state (what runComposeDeploy writes on success) reads deployed.
|
||||||
|
completed := t.TempDir()
|
||||||
|
if err := SaveAppConfig(completed, &AppConfig{
|
||||||
|
Deployed: true, DeployedAt: "2026-06-13T00:00:00Z",
|
||||||
|
Env: map[string]string{"DB_PASSWORD": "x"},
|
||||||
|
}, key, sensitive); err != nil {
|
||||||
|
t.Fatalf("saving completed config: %v", err)
|
||||||
|
}
|
||||||
|
if c := LoadAppConfig(completed); c == nil || !c.Deployed {
|
||||||
|
t.Fatalf("CTRL-T2-1: a completed deploy must read as Deployed=true, got %+v", c)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user