fix(host): a bad CARLOS_RELEASE_PUBKEY says so — unset and unparseable are different problems
domainsTickConfig swallowed the ParsePublicKey error, so a typo'd key logged "CARLOS_RELEASE_PUBKEY is not set" — the one line an operator reads when purging silently is not happening, naming the wrong problem. release.ParsePublicKey says the same sentence for an empty value as for a mistyped one, so logging it verbatim alone would just invert the error. The distinction is drawn before the parser instead: the config builder now returns an advisory error, non-nil only when the variable was SET and would not parse, and domainsLoop logs that verbatim or falls back to "is not set". Either way the Config stays usable and only the auto-purge stands down — hiding a trashed app is reversible, destroying it is not.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 files changed,
+64
−17
internal/host/agent.go+27 −7internal/host/host_test.go+37 −10
diff --git a/internal/host/agent.go b/internal/host/agent.go| index e9b50ca04..26a24b047 100644 |
| --- a/internal/host/agent.go |
| +++ b/internal/host/agent.go |
| @@ -389,10 +389,18 @@ func peekAcmeCache(args []string) string { |
| // nil so DNS stamping stands down: with nothing to compare a resolution |
| // against, "pointing" vs "elsewhere" would be a guess, and unknowns are |
| // never guessed. |
| -func domainsTickConfig(getenv func(string) string, publicIP string) domains.Config { |
| +// |
| +// The error return is ADVISORY, never fatal: it is non-nil only when |
| +// CARLOS_RELEASE_PUBKEY was SET but would not parse, and it exists so the |
| +// caller can log the real cause. release.ParsePublicKey says the same sentence |
| +// for an empty value as for a typo'd one, so an unset key is reported as nil |
| +// error and a nil ReleasePub — the two are different operator problems and the |
| +// one log line about them must not name the wrong one. The Config is fully |
| +// usable either way; only the auto-purge is off. |
| +func domainsTickConfig(getenv func(string) string, publicIP string) (domains.Config, error) { |
| domain := strings.TrimSpace(getenv("PLATFORM_APPS_DOMAIN")) |
| if domain == "" { |
| - return domains.Config{} |
| + return domains.Config{}, nil |
| } |
| var ips []string |
| for _, ip := range strings.Split(getenv("PLATFORM_EDGE_IPS"), ",") { |
| @@ -423,11 +431,18 @@ func domainsTickConfig(getenv func(string) string, publicIP string) domains.Conf |
| // Absence is TOLERATED and leaves ReleasePub nil, which switches the |
| // auto-purge off: a box that cannot verify an app's production flag must |
| // not destroy its objects. Everything else about the pass — including |
| - // hiding trashed apps, the reversible half — carries on regardless. |
| - if pub, err := release.ParsePublicKey(getenv("CARLOS_RELEASE_PUBKEY"), "CARLOS_RELEASE_PUBKEY"); err == nil { |
| + // hiding trashed apps, the reversible half — carries on regardless. A key |
| + // that is SET but unparseable is tolerated the same way, and RETURNED, so |
| + // the operator who typo'd it hears about the typo rather than being told |
| + // the variable is unset. |
| + if raw := strings.TrimSpace(getenv("CARLOS_RELEASE_PUBKEY")); raw != "" { |
| + pub, err := release.ParsePublicKey(raw, "CARLOS_RELEASE_PUBKEY") |
| + if err != nil { |
| + return cfg, err |
| + } |
| cfg.ReleasePub = pub |
| } |
| - return cfg |
| + return cfg, nil |
| } |
| // resolveHost is the production DNS seam: the default resolver with a hard |
| @@ -500,7 +515,7 @@ func domainsLoop(reg *registry.Registry, acmeCacheFlag string) { |
| log.Printf("agent: domains loop disabled: PLATFORM_APPS_DOMAIN is not set") |
| return |
| } |
| - cfg := domainsTickConfig(os.Getenv, readPublicIP()) |
| + cfg, keyErr := domainsTickConfig(os.Getenv, readPublicIP()) |
| if cfg.AppsDomain == "" { |
| log.Printf("agent: domains loop disabled: PLATFORM_APPS_DOMAIN is not set") |
| return |
| @@ -514,7 +529,12 @@ func domainsLoop(reg *registry.Registry, acmeCacheFlag string) { |
| return |
| } |
| log.Printf("agent: domains loop armed (5min + jitter), apps domain %s, edge IPs %v", cfg.AppsDomain, cfg.EdgeIPs) |
| - if cfg.ReleasePub == nil { |
| + // One line, and it must name the REAL cause: a typo'd key and an absent |
| + // one are different problems with different fixes, and this is the only |
| + // place an operator learns that purging is not happening. |
| + if keyErr != nil { |
| + log.Printf("agent: domains loop auto-purge disabled: %v — trashed apps will be hidden but never destroyed", keyErr) |
| + } else if cfg.ReleasePub == nil { |
| log.Printf("agent: domains loop auto-purge disabled: CARLOS_RELEASE_PUBKEY is not set — trashed apps will be hidden but never destroyed") |
| } |
| delay := time.Minute |
diff --git a/internal/host/host_test.go b/internal/host/host_test.go| index 2f806c5f7..4492bedc6 100644 |
| --- a/internal/host/host_test.go |
| +++ b/internal/host/host_test.go |
| @@ -312,15 +312,18 @@ func TestDomainsTickConfig(t *testing.T) { |
| return func(k string) string { return m[k] } |
| } |
| - if got := domainsTickConfig(env(map[string]string{}), "9.9.9.9"); !reflect.DeepEqual(got, domains.Config{}) { |
| - t.Errorf("unset PLATFORM_APPS_DOMAIN: config = %+v, want zero", got) |
| + if got, err := domainsTickConfig(env(map[string]string{}), "9.9.9.9"); !reflect.DeepEqual(got, domains.Config{}) || err != nil { |
| + t.Errorf("unset PLATFORM_APPS_DOMAIN: config = %+v, err = %v, want zero, nil", got, err) |
| } |
| - got := domainsTickConfig(env(map[string]string{ |
| + got, err := domainsTickConfig(env(map[string]string{ |
| "PLATFORM_APPS_DOMAIN": "oncarlos.test", |
| "PLATFORM_EDGE_IPS": "203.0.113.7, 198.51.100.9", |
| "CARLOS_DATA_DIR": "/custom", |
| }), "9.9.9.9") |
| + if err != nil { |
| + t.Errorf("configured deployment with no release key: err = %v, want nil", err) |
| + } |
| if got.AppsDomain != "oncarlos.test" { |
| t.Errorf("AppsDomain = %q", got.AppsDomain) |
| } |
| @@ -335,7 +338,7 @@ func TestDomainsTickConfig(t *testing.T) { |
| } |
| // No explicit edge IPs: the box's own public IP (IMDS) is the default. |
| - got = domainsTickConfig(env(map[string]string{"PLATFORM_APPS_DOMAIN": "oncarlos.test"}), "9.9.9.9") |
| + got, _ = domainsTickConfig(env(map[string]string{"PLATFORM_APPS_DOMAIN": "oncarlos.test"}), "9.9.9.9") |
| if !reflect.DeepEqual(got.EdgeIPs, []string{"9.9.9.9"}) { |
| t.Errorf("EdgeIPs = %v, want the public-IP fallback", got.EdgeIPs) |
| } |
| @@ -345,7 +348,7 @@ func TestDomainsTickConfig(t *testing.T) { |
| // No edge IPs knowable at all: DNS stamping stands down (Resolve nil), |
| // but minting still runs — the domain still works, only health is unknown. |
| - got = domainsTickConfig(env(map[string]string{"PLATFORM_APPS_DOMAIN": "oncarlos.test"}), "") |
| + got, _ = domainsTickConfig(env(map[string]string{"PLATFORM_APPS_DOMAIN": "oncarlos.test"}), "") |
| if got.EdgeIPs != nil || got.Resolve != nil { |
| t.Errorf("no known edge IPs: EdgeIPs = %v Resolve nil? %v — stamping must stand down, not guess", got.EdgeIPs, got.Resolve == nil) |
| } |
| @@ -368,10 +371,13 @@ func TestDomainsTickConfigReleasePubKey(t *testing.T) { |
| if err != nil { |
| t.Fatal(err) |
| } |
| - got := domainsTickConfig(env(map[string]string{ |
| + got, err := domainsTickConfig(env(map[string]string{ |
| "PLATFORM_APPS_DOMAIN": "oncarlos.test", |
| "CARLOS_RELEASE_PUBKEY": pubB64, |
| }), "9.9.9.9") |
| + if err != nil { |
| + t.Fatalf("a good key returned err = %v", err) |
| + } |
| want, err := release.ParsePublicKey(pubB64, "test") |
| if err != nil { |
| t.Fatal(err) |
| @@ -380,16 +386,37 @@ func TestDomainsTickConfigReleasePubKey(t *testing.T) { |
| t.Errorf("ReleasePub = %x, want %x", got.ReleasePub, want) |
| } |
| - for _, key := range []string{"", "not-base64!!", base64.StdEncoding.EncodeToString([]byte("short"))} { |
| - got = domainsTickConfig(env(map[string]string{ |
| + // UNSET and UNPARSEABLE both leave purging off, and the caller must be |
| + // able to tell them apart — release.ParsePublicKey says the same sentence |
| + // for both, so the distinction lives here: unset is (nil, nil), a typo is |
| + // (nil, error). domainsLoop logs the error verbatim, which is the one line |
| + // an operator reads when purging silently is not happening. |
| + for _, key := range []string{"", " "} { |
| + got, err = domainsTickConfig(env(map[string]string{ |
| + "PLATFORM_APPS_DOMAIN": "oncarlos.test", |
| + "CARLOS_RELEASE_PUBKEY": key, |
| + }), "9.9.9.9") |
| + if got.ReleasePub != nil || err != nil { |
| + t.Errorf("CARLOS_RELEASE_PUBKEY=%q: ReleasePub = %x, err = %v — unset must be silent, not an error", key, got.ReleasePub, err) |
| + } |
| + } |
| + for _, key := range []string{"not-base64!!", base64.StdEncoding.EncodeToString([]byte("short"))} { |
| + got, err = domainsTickConfig(env(map[string]string{ |
| "PLATFORM_APPS_DOMAIN": "oncarlos.test", |
| "CARLOS_RELEASE_PUBKEY": key, |
| }), "9.9.9.9") |
| if got.ReleasePub != nil { |
| t.Errorf("CARLOS_RELEASE_PUBKEY=%q armed purging with %x", key, got.ReleasePub) |
| } |
| - if got.AppsDomain != "oncarlos.test" { |
| - t.Errorf("CARLOS_RELEASE_PUBKEY=%q disabled the whole pass", key) |
| + if err == nil { |
| + t.Errorf("CARLOS_RELEASE_PUBKEY=%q returned no error — a typo would be logged as \"not set\"", key) |
| + } else if !strings.Contains(err.Error(), "CARLOS_RELEASE_PUBKEY") { |
| + t.Errorf("CARLOS_RELEASE_PUBKEY=%q: err = %v, want it to name the variable", key, err) |
| + } |
| + // The rest of the pass is unaffected: hiding trashed apps is |
| + // reversible and must not stop because a key is mistyped. |
| + if got.AppsDomain != "oncarlos.test" || got.Resolve == nil { |
| + t.Errorf("CARLOS_RELEASE_PUBKEY=%q disabled the whole pass: %+v", key, got) |
| } |
| } |
| } |