From edc02a4aac4a03c2d9ae50e303becbfe11182ae4 Mon Sep 17 00:00:00 2001 From: Hee Sung Son Date: Mon, 10 Aug 2026 19:13:42 +0900 Subject: [PATCH 1/7] feat(relay-server): report configuration state instead of failing silently Configuration problems in a relay deployment are hard to see. A renamed or mistyped variable looks set but is never read, and a value that cannot be parsed falls back to its default without a word. Both failures look identical to a working deployment until something is missing much later. Add a config subcommand and a startup report that make the effective configuration visible, and stop the silent fallbacks that hid it. - utils: record the env name, flag, usage text and default behind every Flag*Env call. The helpers already received all of it and discarded it, so the descriptions keep a single owner in the flag definitions. - utils: a value that is present but unusable is recorded as an issue rather than swallowed. DISCOVERY=yes is now a startup error instead of a silent false, and an out-of-range port is reported rather than clamped. - relay-server config: prints every key with its effective value, its source, the component that reads it, and its usage text; names keys nothing reads with a nearest-match suggestion; and evaluates each feature. Runs without starting the server, so a deployment can be checked before it is applied. - Startup logs the same report. Each feature is enabled, disabled or blocked: "you switched this off" and "you switched this on but it cannot run" were previously indistinguishable, and only the second is a misconfiguration. Blocked and unprotected states log at warning level. - envcatalog: keys read by Compose, the Google Cloud SDK or the image are catalogued so they are not reported as unknown, and keys the bundled topology pins are marked so documenting them cannot invite an override. - .env.example is grouped by what an operator actually has to decide, and make check-env-example fails when a flag is added without documenting it in .env.example and the configuration reference. The blocked conditions are not new rules; they are failure modes already described in flag usage strings and the docs, moved to startup where they are seen. --- .env.example | 171 +++++-- Makefile | 43 +- cmd/relay-server/config.go | 681 +++++++++++++++++++++++++ cmd/relay-server/envcatalog.go | 74 +++ cmd/relay-server/main.go | 56 +- docs/src/routes/configuration/+page.md | 19 + utils/cmd.go | 125 ++++- 7 files changed, 1080 insertions(+), 89 deletions(-) create mode 100644 cmd/relay-server/config.go create mode 100644 cmd/relay-server/envcatalog.go diff --git a/.env.example b/.env.example index 18e3b698..3912da5c 100644 --- a/.env.example +++ b/.env.example @@ -1,66 +1,145 @@ -# Public routing, discovery, and relay identity persistence +# Portal relay configuration. +# +# Copy to .env and edit. Every key here is read either by the relay binary or by +# Docker Compose; nothing else reads this file. +# +# Check a configured file before starting anything. The report gives the +# effective value and source of every key, names any key that nothing reads, and +# says which features are off and what is missing: +# +# docker compose run --rm portal config --env-file /dev/stdin < .env +# +# API_PORT and SNI_PORT are deliberately absent. The bundled topology fixes them +# at 4017 and 443 because the relay reaches its own API listener through its SNI +# router; overriding them breaks that wiring rather than moving it. + + +# ───────────────────────────────────────────────────────────────────────────── +# 1. Required +# ───────────────────────────────────────────────────────────────────────────── + +# Public HTTPS origin browsers and tunnel clients use. Must be publicly +# resolvable when DISCOVERY=true; localhost and other local-only names are +# rejected by public discovery. PORTAL_URL=https://localhost -# Optional directory inside the Portal container containing a custom SPA index.html. -# Leave empty to use the official frontend embedded in the Portal binary. -PORTAL_FRONTEND_DIR= -DISCOVERY=true + +# Bearer token for the admin and policy APIs. Leaving this empty leaves those +# APIs unauthenticated. Generate one with: openssl rand -hex 32 +ADMIN_TOKEN= + + +# ───────────────────────────────────────────────────────────────────────────── +# 2. Storage and ports +# ───────────────────────────────────────────────────────────────────────────── + +# Directory for relay identity, policy state, and certificate material. +# Also the in-container mount point for ./.portal-certs. IDENTITY_PATH=/portal-certs -# Public HTTPS/SNI +# Public and listen UDP port for the relay overlay. Required when DISCOVERY=true. +# Compose publishes this port. WIREGUARD_PORT=51820 -# Set when enabling public UDP or raw TCP lease ports. + +# Inclusive lease port range shared by the UDP and raw TCP transports. +# 0 disables both. Enabling a transport without a range does nothing; the relay +# reports that at startup. Publish the same range in docker-compose.yml when set. MIN_PORT=0 MAX_PORT=0 -UDP_ENABLED=false -TCP_ENABLED=false - -# Supported managed values: cloudflare, gcloud, hetzner, njalla, route53, vultr. -# Reused for ACME DNS-01, managed A records, ECH HTTPS records, and optional ENS DNS automation. -ACME_DNS_PROVIDER= -# Cloudflare API token (required when ACME_DNS_PROVIDER=cloudflare) -CLOUDFLARE_TOKEN= -# Google Cloud DNS settings. (required when ACME_DNS_PROVIDER=gcloud) -GCP_PROJECT_ID= -GCP_MANAGED_ZONE= -GOOGLE_APPLICATION_CREDENTIALS= +# ───────────────────────────────────────────────────────────────────────────── +# 3. Feature toggles - the defaults suit a single private relay +# ───────────────────────────────────────────────────────────────────────────── -# Hetzner DNS settings (required when ACME_DNS_PROVIDER=hetzner) -HETZNER_API_TOKEN= +# Serve relay discovery endpoints and poll discovery peers. Requires a publicly +# reachable PORTAL_URL and an open WIREGUARD_PORT/udp. +DISCOVERY=true -# Route53 settings (required when ACME_DNS_PROVIDER=route53) -AWS_ACCESS_KEY_ID= -AWS_SECRET_ACCESS_KEY= -AWS_SESSION_TOKEN= -AWS_REGION= -AWS_DEFAULT_REGION= -AWS_HOSTED_ZONE_ID= -# Required only when ACME_DNS_PROVIDER=route53 and ENS_GASLESS_ENABLED=true and no ACTIVE KSK already exists. -AWS_DNSSEC_KMS_KEY_ARN= +# Comma-separated relay API URLs to seed discovery from. +BOOTSTRAPS= -# Vultr DNS settings (required when ACME_DNS_PROVIDER=vultr) -VULTR_API_KEY= +# Enable the UDP and raw TCP lease transports. Both need MIN_PORT/MAX_PORT. +UDP_ENABLED=false +TCP_ENABLED=false -# Njalla DNS settings (required when ACME_DNS_PROVIDER=njalla) -NJALLA_TOKEN= +# Initial landing-page state. Admin changes are persisted in policy.json. +LANDING_PAGE_ENABLED=false -# ENS gasless DNS import automation. When enabled, Portal uses ACME_DNS_PROVIDER -# for DNSSEC and ENS TXT automation, even when certificate files are managed manually. -ENS_GASLESS_ENABLED=false +# Directory inside the container holding a custom SPA index.html. Leave empty to +# serve the frontend embedded in the binary. Uncomment the matching mount in +# docker-compose.yml when setting this. +PORTAL_FRONTEND_DIR= -# Admin/auth configuration. Use a long random value for production relays. -ADMIN_TOKEN= +# Trust X-Forwarded-* and X-Real-IP. Portal owns its public port in the bundled +# topology, so client addresses already come from the socket; enable this only +# when a proxy you control genuinely sits in front. TRUSTED_PROXY_CIDRS empty +# means the default private and loopback ranges. +TRUST_PROXY_HEADERS=false +TRUSTED_PROXY_CIDRS= -# Optional embedded Sui x402 facilitator exposed under /api/x402. +# Relay-owned Sui x402 facilitator under /api/x402. +# X402_ENABLED=true without X402_PAY_TO cannot receive payments. X402_ENABLED=false X402_TESTNET=false X402_PAY_TO= -# Enable only when an explicitly configured upstream proxy supplies forwarded client IP headers. -# Optionally restrict which proxy source ranges may supply those headers; leave empty for default private/loopback proxy ranges. -TRUST_PROXY_HEADERS=false -TRUSTED_PROXY_CIDRS= +# pprof diagnostics. Keep the address on loopback unless the port is otherwise +# protected. PPROF_PORT only matters when you also uncomment the pprof port +# mapping in docker-compose.yml, which exposes it to the host. +PPROF_ENABLED=false +PPROF_ADDR=127.0.0.1:6060 +# PPROF_PORT=6060 -# Initial landing-page state. Admin changes are persisted in policy.json. -LANDING_PAGE_ENABLED=false + +# ───────────────────────────────────────────────────────────────────────────── +# 4. DNS provider - pick one, fill only that block +# +# Used for ACME DNS-01, managed A records, ECH HTTPS records, and optional ENS +# DNS automation. Leave ACME_DNS_PROVIDER empty to place fullchain.pem and +# privatekey.pem under IDENTITY_PATH yourself. +# +# Only one provider's credentials are ever read. The unused blocks below stay +# commented out on purpose. +# ───────────────────────────────────────────────────────────────────────────── + +# cloudflare | gcloud | hetzner | njalla | route53 | vultr +ACME_DNS_PROVIDER= + +# -> ACME_DNS_PROVIDER=cloudflare +# The token needs Zone:Read as well as DNS:Edit. DNS:Edit alone cannot +# locate the zone, and issuance fails with "no cloudflare zone found". +# CLOUDFLARE_TOKEN= + +# -> ACME_DNS_PROVIDER=gcloud +# Project is auto-detected from ADC or GCE metadata when omitted. +# GOOGLE_APPLICATION_CREDENTIALS is read by the Google Cloud SDK itself, +# not by a relay flag: mount the service account file and point this at +# the in-container path. +# GCP_PROJECT_ID= +# GCP_MANAGED_ZONE= +# GOOGLE_APPLICATION_CREDENTIALS= + +# -> ACME_DNS_PROVIDER=hetzner +# HETZNER_API_TOKEN= + +# -> ACME_DNS_PROVIDER=route53 +# Omit the static keys to use the default AWS credential chain. +# AWS_DNSSEC_KMS_KEY_ARN is needed only with ENS_GASLESS_ENABLED=true when +# no ACTIVE KSK exists yet. +# AWS_ACCESS_KEY_ID= +# AWS_SECRET_ACCESS_KEY= +# AWS_SESSION_TOKEN= +# AWS_REGION= +# AWS_HOSTED_ZONE_ID= +# AWS_DNSSEC_KMS_KEY_ARN= + +# -> ACME_DNS_PROVIDER=vultr +# VULTR_API_KEY= + +# -> ACME_DNS_PROVIDER=njalla +# NJALLA_TOKEN= + +# ENS gasless DNS import automation. Requires ACME_DNS_PROVIDER to be set, even +# when certificate files are managed manually. Not needed for normal relay +# operation; leave false unless you specifically want it. +ENS_GASLESS_ENABLED=false diff --git a/Makefile b/Makefile index e2fc9896..d214ce4a 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: help install fmt vet lint lint-auto test tidy all run build build-frontend build-docs build-tunnel build-server build-server-bin clean load-test +.PHONY: help install fmt vet lint lint-auto test tidy all run build build-frontend build-docs build-tunnel build-server build-server-bin clean load-test check-env-example env-reference .DEFAULT_GOAL := help @@ -16,6 +16,8 @@ help: @echo " make fmt - Apply gofmt/goimports" @echo " make lint-auto - Run autofix lint/format pipeline" @echo " make test - Run Go and frontend tests" + @echo " make check-env-example - Fail if .env.example is missing a configuration key" + @echo " make env-reference - Print every configuration key, generated from the flags" @echo " make build - Build Go tunnel and relay server artifacts" @echo " make build-frontend - Build React frontend (Tailwind CSS 4)" @echo " make build-docs - Build documentation site (SvelteKit)" @@ -52,6 +54,45 @@ tidy: go mod tidy go mod verify +# The keys themselves are owned by the flag definitions in +# cmd/relay-server/main.go and by the catalog of keys other components read. +# .env.example and the configuration reference are documentation of that set. +# Adding a flag without documenting it is how configuration drifts away from the +# code, so fail loudly here rather than let an operator find the gap in +# production. Keys the bundled topology pins are excluded on purpose; see +# cmd/relay-server/envcatalog.go. +CONFIG_DOC := docs/src/routes/configuration/+page.md + +check-env-example: + @go run ./cmd/relay-server config --format names > /tmp/portal-env-names.txt + @status=0; \ + missing=""; \ + while read -r name; do \ + grep -qE "^#? *$$name=" .env.example || missing="$$missing $$name"; \ + done < /tmp/portal-env-names.txt; \ + if [ -n "$$missing" ]; then \ + echo "[env] .env.example does not document:"; \ + for name in $$missing; do echo " - $$name"; done; \ + status=1; \ + fi; \ + missing=""; \ + while read -r name; do \ + grep -qF "\`$$name\`" $(CONFIG_DOC) || missing="$$missing $$name"; \ + done < /tmp/portal-env-names.txt; \ + if [ -n "$$missing" ]; then \ + echo "[env] $(CONFIG_DOC) does not document:"; \ + for name in $$missing; do echo " - $$name"; done; \ + status=1; \ + fi; \ + if [ "$$status" -ne 0 ]; then \ + echo "[env] run 'make env-reference' to see each key with its usage text"; \ + exit 1; \ + fi; \ + echo "[env] .env.example and $(CONFIG_DOC) document every configuration key" + +env-reference: + @go run ./cmd/relay-server config --format env + all: fmt vet lint test build run: diff --git a/cmd/relay-server/config.go b/cmd/relay-server/config.go new file mode 100644 index 00000000..d573b187 --- /dev/null +++ b/cmd/relay-server/config.go @@ -0,0 +1,681 @@ +package main + +import ( + "bufio" + "errors" + "flag" + "fmt" + "io" + "net/url" + "os" + "path/filepath" + "slices" + "sort" + "strings" + + "github.com/rs/zerolog/log" + + portalx402 "github.com/gosuda/portal-tunnel/v2/portal/x402" + "github.com/gosuda/portal-tunnel/v2/utils" +) + +// A feature is one capability the operator turned on or off, reported at +// startup and by the config subcommand. Logging the raw settings is not enough: +// "you switched this off" and "you switched this on but it cannot run" both +// show up as a false flag, and only the second one is a misconfiguration. +type featureState string + +const ( + stateEnabled featureState = "enabled" + stateDisabled featureState = "disabled" + stateBlocked featureState = "blocked" + stateUnprotected featureState = "UNPROTECTED" +) + +type feature struct { + Name string + // State is what the feature is actually doing, not what was requested. + State featureState + // By is the setting that produced the state, e.g. "DISCOVERY=true". + By string + // Detail adds context for a working feature. + Detail string + // Missing says what has to be supplied, for blocked and unprotected states. + Missing string +} + +func (f feature) needsAttention() bool { + return f.State == stateBlocked || f.State == stateUnprotected +} + +func evaluateFeatures(cfg relayServerConfig) []feature { + return []feature{ + discoveryFeature(cfg), + acmeFeature(cfg), + ensGaslessFeature(cfg), + leaseTransportFeature("udp-transport", "UDP_ENABLED", cfg.UDPEnabled, cfg), + leaseTransportFeature("tcp-transport", "TCP_ENABLED", cfg.TCPEnabled, cfg), + adminAPIFeature(cfg), + frontendFeature(cfg), + landingPageFeature(cfg), + proxyHeaderFeature(cfg), + x402Feature(cfg), + pprofFeature(cfg), + } +} + +func frontendFeature(cfg relayServerConfig) feature { + f := feature{Name: "frontend"} + dir := strings.TrimSpace(cfg.FrontendDir) + if dir == "" { + f.State, f.By = stateEnabled, "PORTAL_FRONTEND_DIR=" + f.Detail = "serving the SPA embedded in the binary" + return f + } + index := filepath.Join(dir, "index.html") + if _, err := os.Stat(index); err != nil { + f.State, f.By = stateBlocked, "PORTAL_FRONTEND_DIR="+dir + f.Missing = fmt.Sprintf("%s is not readable (%v); mount the directory or clear the variable to use the embedded SPA", index, err) + return f + } + f.State, f.By = stateEnabled, "PORTAL_FRONTEND_DIR="+dir + f.Detail = "serving a custom SPA instead of the embedded one" + return f +} + +func landingPageFeature(cfg relayServerConfig) feature { + f := feature{Name: "landing-page"} + if !cfg.LandingPageEnabled { + f.State, f.By = stateDisabled, "LANDING_PAGE_ENABLED=false" + f.Detail = "the dashboard opens directly on the relay view" + return f + } + f.State, f.By = stateEnabled, "LANDING_PAGE_ENABLED=true" + return f +} + +func discoveryFeature(cfg relayServerConfig) feature { + f := feature{Name: "discovery"} + if !cfg.DiscoveryEnabled { + f.State, f.By = stateDisabled, "DISCOVERY=false" + return f + } + host := portalURLHost(cfg.PortalURL) + if host == "" || utils.IsLocalRelayHost(host) { + f.State, f.By = stateBlocked, "DISCOVERY=true" + f.Missing = fmt.Sprintf( + "PORTAL_URL host %q is local-only and public discovery rejects it; set PORTAL_URL to a publicly resolvable HTTPS origin", + host) + return f + } + f.State, f.By = stateEnabled, "DISCOVERY=true" + f.Detail = fmt.Sprintf("host=%s bootstraps=%d wireguard_port=%d", + host, len(utils.SplitCSV(cfg.Bootstraps)), cfg.WireGuardPort) + return f +} + +func acmeFeature(cfg relayServerConfig) feature { + f := feature{Name: "acme"} + provider := strings.ToLower(strings.TrimSpace(cfg.ACMEDNSProvider)) + if provider == "" { + f.State, f.By = stateDisabled, "ACME_DNS_PROVIDER=" + f.Detail = "manual certificates: place fullchain.pem and privatekey.pem under IDENTITY_PATH" + return f + } + + required, supported := dnsProviderCredential[provider] + if !supported { + f.State, f.By = stateBlocked, "ACME_DNS_PROVIDER="+provider + f.Missing = "unsupported provider; use cloudflare, gcloud, hetzner, njalla, route53 or vultr" + return f + } + + var empty []string + for _, name := range required { + if strings.TrimSpace(providerCredential(cfg, name)) == "" { + empty = append(empty, name) + } + } + if len(empty) > 0 { + f.State, f.By = stateBlocked, "ACME_DNS_PROVIDER="+provider + f.Missing = strings.Join(empty, ", ") + " is empty" + return f + } + + f.State, f.By = stateEnabled, "ACME_DNS_PROVIDER="+provider + f.Detail = "managed issuance and renewal under IDENTITY_PATH" + if len(required) == 0 { + f.Detail += "; credentials come from the ambient provider chain" + } + return f +} + +func ensGaslessFeature(cfg relayServerConfig) feature { + f := feature{Name: "ens-gasless"} + if !cfg.ENSGaslessEnabled { + f.State, f.By = stateDisabled, "ENS_GASLESS_ENABLED=false" + return f + } + if strings.TrimSpace(cfg.ACMEDNSProvider) == "" { + f.State, f.By = stateBlocked, "ENS_GASLESS_ENABLED=true" + f.Missing = "ACME_DNS_PROVIDER is empty; ENS gasless automation needs a DNS provider even when certificates are managed manually" + return f + } + f.State, f.By = stateEnabled, "ENS_GASLESS_ENABLED=true" + f.Detail = "DNSSEC and ENS TXT automation through " + cfg.ACMEDNSProvider + return f +} + +func leaseTransportFeature(name, envName string, enabled bool, cfg relayServerConfig) feature { + f := feature{Name: name} + if !enabled { + f.State, f.By = stateDisabled, envName+"=false" + return f + } + if cfg.MinPort <= 0 || cfg.MaxPort <= 0 || cfg.MaxPort < cfg.MinPort { + f.State, f.By = stateBlocked, envName+"=true" + f.Missing = fmt.Sprintf( + "MIN_PORT=%d MAX_PORT=%d is not a usable range; set both and publish the range in docker-compose.yml", + cfg.MinPort, cfg.MaxPort) + return f + } + f.State, f.By = stateEnabled, envName+"=true" + f.Detail = fmt.Sprintf("ports=%d-%d", cfg.MinPort, cfg.MaxPort) + return f +} + +func adminAPIFeature(cfg relayServerConfig) feature { + f := feature{Name: "admin-api"} + if strings.TrimSpace(cfg.AdminToken) == "" { + f.State = stateUnprotected + f.Missing = "ADMIN_TOKEN is empty; the admin and policy APIs accept unauthenticated requests. Generate one with: openssl rand -hex 32" + return f + } + f.State, f.By = stateEnabled, "ADMIN_TOKEN set" + f.Detail = "bearer token required for /api/admin and /api/policy" + return f +} + +func proxyHeaderFeature(cfg relayServerConfig) feature { + f := feature{Name: "proxy-headers"} + if !cfg.TrustProxyHeaders { + f.State, f.By = stateDisabled, "TRUST_PROXY_HEADERS=false" + f.Detail = "client addresses come from the socket, which is correct when Portal owns the public port itself" + return f + } + f.State, f.By = stateEnabled, "TRUST_PROXY_HEADERS=true" + if cidrs := strings.TrimSpace(cfg.TrustedProxyCIDRs); cidrs != "" { + f.Detail = "trusted=" + cidrs + } else { + f.Detail = "trusted=default private and loopback ranges (TRUSTED_PROXY_CIDRS empty)" + } + return f +} + +func x402Feature(cfg relayServerConfig) feature { + f := feature{Name: "x402"} + if !cfg.X402Enabled { + f.State, f.By = stateDisabled, "X402_ENABLED=false" + return f + } + if strings.TrimSpace(cfg.X402PayTo) == "" { + f.State, f.By = stateBlocked, "X402_ENABLED=true" + f.Missing = "X402_PAY_TO is empty; the facilitator has no payment recipient" + return f + } + f.State, f.By = stateEnabled, "X402_ENABLED=true" + f.Detail = "network=" + portalx402.Network(cfg.X402Testnet) + return f +} + +func pprofFeature(cfg relayServerConfig) feature { + f := feature{Name: "pprof"} + if !cfg.PProfEnabled { + f.State, f.By = stateDisabled, "PPROF_ENABLED=false" + return f + } + f.State, f.By = stateEnabled, "PPROF_ENABLED=true" + f.Detail = "addr=" + cfg.PProfAddr + return f +} + +func portalURLHost(raw string) string { + parsed, err := url.Parse(strings.TrimSpace(raw)) + if err != nil { + return "" + } + return utils.NormalizeHostname(parsed.Hostname()) +} + +func providerCredential(cfg relayServerConfig, name string) string { + switch name { + case "CLOUDFLARE_TOKEN": + return cfg.CloudflareToken + case "HETZNER_API_TOKEN": + return cfg.HetznerAPIToken + case "NJALLA_TOKEN": + return cfg.NjallaToken + case "VULTR_API_KEY": + return cfg.VultrAPIKey + default: + return "" + } +} + +// logFeatureReport emits the same report the config subcommand renders, so the +// two can never describe the deployment differently. +func logFeatureReport(features []feature) { + for _, f := range features { + event := log.Info() + if f.needsAttention() { + event = log.Warn() + } + event = event.Str("feature", f.Name).Str("state", string(f.State)) + if f.By != "" { + event = event.Str("by", f.By) + } + if f.Detail != "" { + event = event.Str("detail", f.Detail) + } + if f.Missing != "" { + event = event.Str("missing", f.Missing) + } + event.Msg("feature") + } +} + +// envFileEntry is one assignment read from an env file, kept in file order so +// the report follows the operator's own layout. +type envFileEntry struct { + Name string + Value string +} + +func loadEnvFile(path string) ([]envFileEntry, error) { + file, err := os.Open(path) + if err != nil { + return nil, err + } + defer func() { _ = file.Close() }() + + var entries []envFileEntry + scanner := bufio.NewScanner(file) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + line = strings.TrimPrefix(line, "export ") + name, value, found := strings.Cut(line, "=") + if !found { + continue + } + name = strings.TrimSpace(name) + if name == "" { + continue + } + // Compose does not expand values read from an env file, so neither do we. + value = strings.TrimSpace(value) + if len(value) >= 2 && (value[0] == '"' || value[0] == '\'') && value[len(value)-1] == value[0] { + value = value[1 : len(value)-1] + } + entries = append(entries, envFileEntry{Name: name, Value: value}) + } + if err := scanner.Err(); err != nil { + return nil, err + } + return entries, nil +} + +// knownEnvNames indexes every name the relay reads, including flag aliases. +func knownEnvNames() map[string]utils.EnvVar { + index := make(map[string]utils.EnvVar) + for _, entry := range utils.EnvVars() { + index[entry.Name] = entry + for _, alias := range entry.Aliases { + index[alias] = entry + } + } + return index +} + +func secretEnvName(name string) bool { + upper := strings.ToUpper(name) + for _, marker := range []string{"TOKEN", "SECRET", "KEY", "PASSWORD", "CREDENTIALS"} { + if strings.Contains(upper, marker) { + return true + } + } + return false +} + +func displayValue(name, value string) string { + if strings.TrimSpace(value) == "" { + return "" + } + if secretEnvName(name) { + return "" + } + return value +} + +func writeConfigReport(w io.Writer, cfg relayServerConfig, entries []envFileEntry, source string) { + relay := knownEnvNames() + + fmt.Fprintf(w, "Portal relay configuration (%s)\n\n", source) + + if len(entries) > 0 { + fmt.Fprintln(w, "Keys") + var unknown []envFileEntry + for _, entry := range entries { + switch { + case relay[entry.Name].Name != "": + known := relay[entry.Name] + fmt.Fprintf(w, " OK %-30s %-24s relay --%s\n", + entry.Name, displayValue(entry.Name, entry.Value), known.Flag) + writeWrapped(w, known.Usage) + if pinned, ok := pinnedByTopology[entry.Name]; ok && entry.Value != pinned.Value { + fmt.Fprintf(w, " WARNING: pinned to %s by the bundled topology; %s\n", + pinned.Value, pinned.Reason) + } + if note := alsoConsumedBy[entry.Name]; note != "" { + fmt.Fprintf(w, " also: %s\n", note) + } + case externalEnvVars[entry.Name].Owner != "": + external := externalEnvVars[entry.Name] + fmt.Fprintf(w, " OK %-30s %-24s %s\n", + entry.Name, displayValue(entry.Name, entry.Value), external.Owner) + writeWrapped(w, external.Usage) + default: + unknown = append(unknown, entry) + } + } + + if len(unknown) > 0 { + fmt.Fprintf(w, "\nUNKNOWN %d key(s) are not read by any component and are silently ignored:\n", len(unknown)) + for _, entry := range unknown { + if suggestion := nearestEnvName(entry.Name, relay); suggestion != "" { + fmt.Fprintf(w, " %-30s did you mean %s?\n", entry.Name, suggestion) + continue + } + fmt.Fprintf(w, " %-30s no equivalent key exists\n", entry.Name) + } + } + fmt.Fprintln(w) + } + + fmt.Fprintln(w, "Features") + for _, f := range evaluateFeatures(cfg) { + marker := " " + if f.needsAttention() { + marker = "!" + } + fmt.Fprintf(w, " %s %-16s %-12s %s\n", marker, f.Name, f.State, f.By) + if f.Detail != "" { + writeWrapped(w, f.Detail) + } + if f.Missing != "" { + writeWrapped(w, "missing: "+f.Missing) + } + } + + if issues := utils.EnvIssues(); len(issues) > 0 { + fmt.Fprintln(w, "\nInvalid values") + for _, issue := range issues { + fmt.Fprintf(w, " %s=%s %s\n", issue.Name, issue.Value, issue.Problem) + } + } +} + +// writeWrapped prints an indented, soft-wrapped continuation line. +func writeWrapped(w io.Writer, text string) { + text = strings.TrimSpace(text) + if text == "" { + return + } + const width = 72 + const indent = " " + line := indent + for _, word := range strings.Fields(text) { + if len(line)+len(word)+1 > width && strings.TrimSpace(line) != "" { + fmt.Fprintln(w, line) + line = indent + } + if strings.TrimSpace(line) == "" { + line += word + continue + } + line += " " + word + } + if strings.TrimSpace(line) != "" { + fmt.Fprintln(w, line) + } +} + +// nearestEnvName suggests the closest known key for a typo. Deployment drift +// usually looks like ADMIN_WALLETS for ADMIN_TOKEN: close enough to look right, +// far enough that nothing reads it. +func nearestEnvName(name string, relay map[string]utils.EnvVar) string { + candidates := make([]string, 0, len(relay)+len(externalEnvVars)) + for candidate := range relay { + candidates = append(candidates, candidate) + } + for candidate := range externalEnvVars { + candidates = append(candidates, candidate) + } + sort.Strings(candidates) + + best := "" + bestDistance := len(name)/2 + 2 + for _, candidate := range candidates { + if distance := editDistance(name, candidate); distance < bestDistance { + best, bestDistance = candidate, distance + } + } + return best +} + +func editDistance(a, b string) int { + previous := make([]int, len(b)+1) + current := make([]int, len(b)+1) + for j := range previous { + previous[j] = j + } + for i := 1; i <= len(a); i++ { + current[0] = i + for j := 1; j <= len(b); j++ { + cost := 1 + if a[i-1] == b[j-1] { + cost = 0 + } + current[j] = min(previous[j]+1, current[j-1]+1, previous[j-1]+cost) + } + previous, current = current, previous + } + return previous[len(b)] +} + +// writeEnvReference emits every key the deployment understands, grouped by +// owner. It is generated from the flag definitions and the catalog, so it +// cannot drift from the code the way a hand-written list does. +func writeEnvReference(w io.Writer) { + fmt.Fprintln(w, "# Generated by `relay-server config --format env`. Do not edit by hand.") + fmt.Fprintln(w, "# Every key the bundled deployment understands, grouped by the component") + fmt.Fprintln(w, "# that reads it. See .env.example for a commented starting point.") + + fmt.Fprintln(w, "\n# ── relay ──") + for _, entry := range utils.EnvVars() { + fmt.Fprintf(w, "\n# %s [relay --%s] default: %s\n", entry.Name, entry.Flag, defaultDisplay(entry.Default)) + if len(entry.Aliases) > 0 { + fmt.Fprintf(w, "# also accepted: %s\n", strings.Join(entry.Aliases, ", ")) + } + writeCommentWrapped(w, entry.Usage) + fmt.Fprintf(w, "%s=%s\n", entry.Name, entry.Default) + } + + owners := make([]string, 0, len(externalEnvVars)) + byOwner := map[string][]string{} + for name, external := range externalEnvVars { + if _, seen := byOwner[external.Owner]; !seen { + owners = append(owners, external.Owner) + } + byOwner[external.Owner] = append(byOwner[external.Owner], name) + } + sort.Strings(owners) + for _, owner := range owners { + names := byOwner[owner] + sort.Strings(names) + fmt.Fprintf(w, "\n# ── %s ──\n", owner) + for _, name := range names { + fmt.Fprintf(w, "\n# %s [%s]\n", name, owner) + writeCommentWrapped(w, externalEnvVars[name].Usage) + fmt.Fprintf(w, "# %s=\n", name) + } + } +} + +// writeEnvNames lists the keys an operator is expected to set, one per line, so +// `make check-env-example` can assert .env.example still documents all of them. +// A flag added without a matching .env.example entry is exactly how the +// documented configuration drifts away from the code. +// +// Keys the bundled topology pins in the image are excluded: they are recognised +// everywhere else, but documenting them would invite an override that breaks +// the wiring between nginx and the services behind it. +func writeEnvNames(w io.Writer) { + names := make([]string, 0, len(externalEnvVars)) + for _, entry := range utils.EnvVars() { + if _, pinned := pinnedByTopology[entry.Name]; pinned { + continue + } + names = append(names, entry.Name) + } + for name, external := range externalEnvVars { + if external.Pinned { + continue + } + names = append(names, name) + } + sort.Strings(names) + for _, name := range slices.Compact(names) { + fmt.Fprintln(w, name) + } +} + +func defaultDisplay(value string) string { + if value == "" { + return "(empty)" + } + return value +} + +func writeCommentWrapped(w io.Writer, text string) { + text = strings.TrimSpace(text) + if text == "" { + return + } + const width = 74 + line := "# " + for _, word := range strings.Fields(text) { + if len(line)+len(word)+1 > width && strings.TrimSpace(line) != "#" { + fmt.Fprintln(w, line) + line = "# " + } + if line == "# " { + line += word + continue + } + line += " " + word + } + if strings.TrimSpace(line) != "#" { + fmt.Fprintln(w, line) + } +} + +func runConfigCommand(args []string) error { + var ( + envFilePath string + format string + ) + fs := utils.NewFlagSet("relay-server config", printConfigUsage) + utils.StringFlag(fs, &envFilePath, "env-file", "", "env file to inspect instead of the process environment") + utils.StringFlag(fs, &format, "format", "text", "output format: text, env or names") + + if err := utils.ParseFlagSet(fs, args, printConfigUsage); err != nil { + if errors.Is(err, flag.ErrHelp) { + return nil + } + return err + } + if err := utils.RequireNoArgs(fs.Args(), "relay-server config"); err != nil { + printConfigUsage(os.Stderr) + return err + } + + // Load the file before registering flags: flag defaults resolve from the + // process environment, which is exactly how Compose delivers env_file. + var entries []envFileEntry + source := "process environment" + if strings.TrimSpace(envFilePath) != "" { + loaded, err := loadEnvFile(envFilePath) + if err != nil { + return fmt.Errorf("read env file: %w", err) + } + for _, entry := range loaded { + if err := os.Setenv(entry.Name, entry.Value); err != nil { + return fmt.Errorf("apply %s: %w", entry.Name, err) + } + } + entries = loaded + source = envFilePath + } + + cfg, err := resolveRelayServerConfig(nil) + if err != nil { + return err + } + + switch strings.TrimSpace(format) { + case "", "text": + writeConfigReport(os.Stdout, cfg, entries, source) + return nil + case "env": + writeEnvReference(os.Stdout) + return nil + case "names": + writeEnvNames(os.Stdout) + return nil + default: + printConfigUsage(os.Stderr) + return fmt.Errorf("unknown format %q", format) + } +} + +func printConfigUsage(w io.Writer) { + utils.WriteCommandUsage(w, + []string{ + "relay-server config [--env-file PATH] [--format text|env]", + }, + []string{ + "relay-server config", + "relay-server config --env-file .env", + "relay-server config --format env > env.reference", + }, + ) +} + +// envIssueError turns recorded parse failures into a startup error. A value +// that cannot be parsed is always a mistake, and falling back silently is what +// let deployments run for months with settings nothing read. +func envIssueError() error { + issues := utils.EnvIssues() + if len(issues) == 0 { + return nil + } + messages := make([]string, 0, len(issues)) + for _, issue := range issues { + messages = append(messages, fmt.Sprintf("%s=%q: %s", issue.Name, issue.Value, issue.Problem)) + } + slices.Sort(messages) + return errors.New("invalid environment values: " + strings.Join(messages, "; ")) +} diff --git a/cmd/relay-server/envcatalog.go b/cmd/relay-server/envcatalog.go new file mode 100644 index 00000000..a26d59ab --- /dev/null +++ b/cmd/relay-server/envcatalog.go @@ -0,0 +1,74 @@ +package main + +// The deployment .env is shared by the relay and by Docker Compose itself. +// Checking a key against the relay's own flags alone would report the +// Compose-level ones as unknown, so the keys owned elsewhere are catalogued +// here. +// +// This table is the only place that knowledge lives. Relay-owned keys are not +// listed: they come from the flag definitions in main.go through +// utils.EnvVars(). + +const ( + ownerCompose = "compose" + ownerGoogleSDK = "Google Cloud SDK" + ownerImage = "container image" +) + +// externalEnvVar is a deployment key this binary does not read. +// +// Pinned keys are recognised so that setting one is never reported as unknown, +// but they are left out of .env.example on purpose: the bundled topology fixes +// them, and listing them invites an override that breaks the wiring. Pinned is +// what keeps that decision from silently reverting the next time someone runs +// the drift check. +type externalEnvVar struct { + Owner string + Usage string + Pinned bool +} + +var externalEnvVars = map[string]externalEnvVar{ + "PPROF_PORT": {Owner: ownerCompose, + Usage: "host port published for the pprof listener, when that mapping is uncommented in docker-compose.yml. Consumed when Compose parses the file, so it is not a container variable."}, + + "GOOGLE_APPLICATION_CREDENTIALS": {Owner: ownerGoogleSDK, + Usage: "service account file path. Read by the Google Cloud SDK directly rather than by a relay flag, so it is passed through untouched."}, + + "TZ": {Owner: ownerImage, Pinned: true, + Usage: "container time zone. Set to UTC by the image."}, +} + +// alsoConsumedBy notes extra consumers of keys the relay does read, so the +// report can say that changing one moves more than the relay. +var alsoConsumedBy = map[string]string{ + "IDENTITY_PATH": "compose mounts ./.portal-certs at this path", + "WIREGUARD_PORT": "compose publishes this UDP port", + "MIN_PORT": "compose publishes this port range when the mapping is uncommented", + "MAX_PORT": "compose publishes this port range when the mapping is uncommented", + "PORTAL_FRONTEND_DIR": "compose has a matching read-only mount to uncomment when replacing the SPA", +} + +// pinnedByTopology are keys the bundled Compose stack fixes because the relay +// reaches itself at those ports through its own SNI router. Overriding one +// through .env breaks that wiring, so the report calls it out. +var pinnedByTopology = map[string]struct { + Value string + Reason string +}{ + "API_PORT": {"4017", "the SNI router forwards root-host traffic to the internal API listener on this port"}, + "SNI_PORT": {"443", "this is the public port tunnel clients are told to reach"}, +} + +// dnsProviderCredential maps each supported ACME_DNS_PROVIDER value to the +// credential it requires. Providers whose credentials come from an ambient +// chain (an instance role, application default credentials) map to an empty +// list because there is nothing to require. +var dnsProviderCredential = map[string][]string{ + "cloudflare": {"CLOUDFLARE_TOKEN"}, + "hetzner": {"HETZNER_API_TOKEN"}, + "njalla": {"NJALLA_TOKEN"}, + "vultr": {"VULTR_API_KEY"}, + "route53": nil, + "gcloud": nil, +} diff --git a/cmd/relay-server/main.go b/cmd/relay-server/main.go index d97842b3..ea1de30a 100644 --- a/cmd/relay-server/main.go +++ b/cmd/relay-server/main.go @@ -24,9 +24,10 @@ import ( func main() { log.Logger = log.Output(zerolog.NewConsoleWriter()) if err := utils.RunCommands(os.Args[1:], os.Stdout, os.Stderr, printRootUsage, map[string]utils.CommandFunc{ - "": runServeCommand, - "serve": runServeCommand, - "help": runHelpCommand, + "": runServeCommand, + "serve": runServeCommand, + "config": runConfigCommand, + "help": runHelpCommand, }); err != nil { log.Error().Err(err).Msg("execute root command") os.Exit(1) @@ -72,7 +73,10 @@ type relayServerConfig struct { NjallaToken string } -func runServeCommand(args []string) error { +// resolveRelayServerConfig registers every flag and resolves it against the +// process environment. The config subcommand reuses it so that inspecting a +// deployment and running it read the same definitions. +func resolveRelayServerConfig(args []string) (relayServerConfig, error) { cfg := relayServerConfig{} fs := utils.NewFlagSet("relay-server", printRootUsage) @@ -117,43 +121,41 @@ func runServeCommand(args []string) error { utils.StringFlagEnv(fs, &cfg.NjallaToken, "njalla-token", "", "Njalla API token for DNS automation (required when acme-dns-provider=njalla)", "NJALLA_TOKEN") if err := utils.ParseFlagSet(fs, args, printRootUsage); err != nil { + return relayServerConfig{}, err + } + if err := utils.RequireNoArgs(fs.Args(), "relay-server"); err != nil { + printRootUsage(os.Stderr) + return relayServerConfig{}, err + } + cfg.IdentityPath = identity.ResolveRelayStateDir(cfg.IdentityPath) + return cfg, nil +} + +func runServeCommand(args []string) error { + cfg, err := resolveRelayServerConfig(args) + if err != nil { if errors.Is(err, flag.ErrHelp) { return nil } return err } - if err := utils.RequireNoArgs(fs.Args(), "relay-server"); err != nil { - printRootUsage(os.Stderr) + // A value that could not be parsed is always a mistake. Starting anyway is + // how a deployment ends up running with a setting nobody reads. + if err := envIssueError(); err != nil { return err } - cfg.IdentityPath = identity.ResolveRelayStateDir(cfg.IdentityPath) log.Info(). Str("release_version", types.ReleaseVersion). Str("portal_url", cfg.PortalURL). - Str("frontend_dir", cfg.FrontendDir). Str("identity_path", cfg.IdentityPath). - Str("bootstraps", cfg.Bootstraps). - Bool("discovery_enabled", cfg.DiscoveryEnabled). - Int("wireguard_port", cfg.WireGuardPort). Int("api_port", cfg.APIPort). Int("sni_port", cfg.SNIPort). - Bool("trust_proxy_headers", cfg.TrustProxyHeaders). - Str("trusted_proxy_cidrs", cfg.TrustedProxyCIDRs). - Bool("udp_enabled", cfg.UDPEnabled). - Bool("tcp_enabled", cfg.TCPEnabled). - Bool("landing_page_enabled", cfg.LandingPageEnabled). - Int("min_port", cfg.MinPort). - Int("max_port", cfg.MaxPort). - Bool("admin_token_configured", strings.TrimSpace(cfg.AdminToken) != ""). - Bool("pprof_enabled", cfg.PProfEnabled). - Str("pprof_addr", cfg.PProfAddr). - Bool("x402_facilitator_enabled", cfg.X402Enabled). - Bool("x402_testnet", cfg.X402Testnet). - Bool("x402_pay_to_configured", strings.TrimSpace(cfg.X402PayTo) != ""). - Str("acme_dns_provider", cfg.ACMEDNSProvider). - Bool("ens_gasless_enabled", cfg.ENSGaslessEnabled). - Msg("configured relay server") + Msg("starting relay server") + + // Report each capability with the setting that produced it, so a feature + // that was switched off is distinguishable from one that cannot run. + logFeatureReport(evaluateFeatures(cfg)) ctx, stop := utils.SignalContext() defer stop() diff --git a/docs/src/routes/configuration/+page.md b/docs/src/routes/configuration/+page.md index 3c80bd4c..a4268ca8 100644 --- a/docs/src/routes/configuration/+page.md +++ b/docs/src/routes/configuration/+page.md @@ -7,10 +7,28 @@ description: Complete reference for all Portal environment variables, CLI flags, Complete reference for all Portal environment variables, CLI flags, and configuration files. +## Checking a Real Deployment + +This page describes what each variable means. To see what a specific deployment +is actually doing, ask the binary rather than reading a table: + +```bash +relay-server config --env-file .env +``` + +It prints every key with its effective value and where that value came from, +names any key nothing reads, and reports which features are off and what is +missing. `relay-server config --format env` regenerates the full list from the +flag definitions, and `make check-env-example` fails when this page or +`.env.example` stops mentioning a key. + ## Relay Server Environment Variables The relay server (`relay-server`) reads configuration from environment variables. Each variable corresponds to a CLI flag of the same shape (e.g. `PORTAL_URL` → `--portal-url`). CLI flags take precedence over environment variables when both are set. +A value that cannot be parsed is a startup error rather than a silent fallback: +`DISCOVERY=yes` fails immediately instead of resolving to `false`. + ### Core | Variable | Default | Type | Description | @@ -67,6 +85,7 @@ The relay server (`relay-server`) reads configuration from environment variables |----------|---------|------|-------------| | `PPROF_ENABLED` | `false` | bool | Enable the relay pprof diagnostics HTTP server | | `PPROF_ADDR` | `127.0.0.1:6060` | string | pprof listen address when enabled; keep it on loopback unless the port is protected | +| `PPROF_PORT` | `6060` | int | Host port published for the pprof listener, read by Docker Compose rather than the relay. Only takes effect when the matching port mapping in `docker-compose.yml` is uncommented, which exposes it to the host | ### Admin diff --git a/utils/cmd.go b/utils/cmd.go index 36c8da0e..77a780a1 100644 --- a/utils/cmd.go +++ b/utils/cmd.go @@ -9,6 +9,7 @@ import ( "net" "os" "os/signal" + "slices" "strconv" "strings" "syscall" @@ -19,22 +20,97 @@ type CommandFunc func([]string) error type IntEnvParser func(string, int) int type boolFlagValue interface{ IsBoolFlag() bool } +// EnvVar records one environment variable that backs a flag, together with the +// flag's own documentation. Flag definitions already carry the name, default, +// and usage text, so registering them here lets `relay-server config`, the +// startup feature report, and the generated .env.example all read from the flag +// definitions instead of a second hand-maintained list. +type EnvVar struct { + Name string + Aliases []string + Flag string + Usage string + Default string + // SetBy is the environment variable that supplied the value, or empty when + // the default was used. It answers "I set that, why did nothing change?". + SetBy string +} + +// EnvIssue is a value that was present but unusable. Without recording these, +// the resolve helpers below silently fall back and a typo is indistinguishable +// from an intentional default. +type EnvIssue struct { + Name string + Value string + Problem string +} + +// The process environment is process-global, so the registry is too. Flag +// registration happens once per process before any concurrent work starts. +var ( + envVars []EnvVar + envVarIndex = map[string]int{} + envIssues []EnvIssue +) + +// EnvVars returns every environment variable backing a registered flag, in +// registration order. +func EnvVars() []EnvVar { + return slices.Clone(envVars) +} + +// EnvIssues returns values that were set but could not be used. +func EnvIssues() []EnvIssue { + return slices.Clone(envIssues) +} + +func registerEnvVar(flagName, usage, defaultValue, setBy string, envNames []string) { + names := make([]string, 0, len(envNames)) + for _, envName := range envNames { + if envName = strings.TrimSpace(envName); envName != "" { + names = append(names, envName) + } + } + if len(names) == 0 { + return + } + + entry := EnvVar{ + Name: names[0], + Aliases: names[1:], + Flag: flagName, + Usage: usage, + Default: defaultValue, + SetBy: setBy, + } + // Registering the same flag twice (a re-parsed command, a test) replaces the + // entry rather than duplicating it. + if i, ok := envVarIndex[entry.Name]; ok { + envVars[i] = entry + return + } + envVarIndex[entry.Name] = len(envVars) + envVars = append(envVars, entry) +} + +func recordEnvIssue(name, value, problem string) { + envIssues = append(envIssues, EnvIssue{Name: name, Value: value, Problem: problem}) +} + func trimmedEnv(name string) string { return strings.TrimSpace(os.Getenv(name)) } -func resolveStringEnv(fallback string, envNames ...string) string { - value := fallback +func resolveStringEnv(fallback string, envNames ...string) (string, string) { for _, envName := range envNames { if envValue := trimmedEnv(envName); envValue != "" { - value = envValue - break + return envValue, envName } } - return value + return fallback, "" } -func resolveBoolEnv(fallback bool, envNames ...string) bool { +func resolveBoolEnv(fallback bool, envNames ...string) (bool, string) { for _, envName := range envNames { raw := trimmedEnv(envName) if raw == "" { @@ -42,14 +118,15 @@ func resolveBoolEnv(fallback bool, envNames ...string) bool { } parsed, err := strconv.ParseBool(raw) if err != nil { - return fallback + recordEnvIssue(envName, raw, "not a boolean; use true or false") + return fallback, "" } - return parsed + return parsed, envName } - return fallback + return fallback, "" } -func resolveIntEnv(fallback int, parse IntEnvParser, envNames ...string) int { +func resolveIntEnv(fallback int, parse IntEnvParser, envNames ...string) (int, string) { if parse == nil { parse = func(raw string, fallback int) int { v, err := strconv.Atoi(strings.TrimSpace(raw)) @@ -64,9 +141,21 @@ func resolveIntEnv(fallback int, parse IntEnvParser, envNames ...string) int { if raw == "" { continue } - return parse(raw, fallback) + // Parse first so a non-numeric value is reported as such instead of + // being flattened into the parser's fallback. + number, err := strconv.Atoi(raw) + if err != nil { + recordEnvIssue(envName, raw, "not an integer") + return fallback, "" + } + value := parse(raw, fallback) + if value != number && value == fallback { + recordEnvIssue(envName, raw, "out of the accepted range") + return fallback, "" + } + return value, envName } - return fallback + return fallback, "" } func ParsePortNumber(raw string, fallback int) int { @@ -118,7 +207,9 @@ func StringFlag(fs *flag.FlagSet, target *string, name, fallback, usage string) } func StringFlagEnv(fs *flag.FlagSet, target *string, name, fallback, usage string, envNames ...string) { - ensureFlagSet(fs).StringVar(target, name, resolveStringEnv(fallback, envNames...), flagUsage(usage, envNames...)) + value, setBy := resolveStringEnv(fallback, envNames...) + registerEnvVar(name, usage, fallback, setBy, envNames) + ensureFlagSet(fs).StringVar(target, name, value, flagUsage(usage, envNames...)) } func BoolFlag(fs *flag.FlagSet, target *bool, name string, fallback bool, usage string) { @@ -126,11 +217,15 @@ func BoolFlag(fs *flag.FlagSet, target *bool, name string, fallback bool, usage } func BoolFlagEnv(fs *flag.FlagSet, target *bool, name string, fallback bool, usage string, envNames ...string) { - ensureFlagSet(fs).BoolVar(target, name, resolveBoolEnv(fallback, envNames...), flagUsage(usage, envNames...)) + value, setBy := resolveBoolEnv(fallback, envNames...) + registerEnvVar(name, usage, strconv.FormatBool(fallback), setBy, envNames) + ensureFlagSet(fs).BoolVar(target, name, value, flagUsage(usage, envNames...)) } func IntFlagEnv(fs *flag.FlagSet, target *int, name string, fallback int, parse IntEnvParser, usage string, envNames ...string) { - ensureFlagSet(fs).IntVar(target, name, resolveIntEnv(fallback, parse, envNames...), flagUsage(usage, envNames...)) + value, setBy := resolveIntEnv(fallback, parse, envNames...) + registerEnvVar(name, usage, strconv.Itoa(fallback), setBy, envNames) + ensureFlagSet(fs).IntVar(target, name, value, flagUsage(usage, envNames...)) } func RepeatedStringFlag(fs *flag.FlagSet, target *[]string, name, usage string) { From 5ef6c73ed94800d5968caf5520cf6cd1c3bf4388 Mon Sep 17 00:00:00 2001 From: Hee Sung Son Date: Tue, 11 Aug 2026 11:37:41 +0900 Subject: [PATCH 2/7] fix(relay-server): report resolved values and keep dependent features in sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three problems found in review. The report printed the raw text of each env-file line rather than the value the flag resolved to. A key overridden by a higher-priority name, or an alias that was never consulted, read as though it were in effect — the exact confusion the report exists to remove. It also skipped the key listing entirely when no env file was given, so a process-environment deployment got no listing at all. The registry now carries the resolved value, and every relay key is listed with it plus where it came from: the env file, the process environment, or the default. When an alias supplied the value the alias is named, so "I set AWS_REGION, why is it different?" is answered by seeing AWS_DEFAULT_REGION was consulted first. Keys owned by another component are still only shown when supplied, since the relay has no effective value for them. ens-gasless repeated only the "is a provider set" half of the ACME check, so it reported enabled while acme was blocked on a missing credential. It now defers to acmeFeature and propagates the reason. The registry is process-global, and the config subcommand resolves a second time after loading an env file. Issues accumulated across passes, so a stale one could fail a configuration that no longer contained it. Resolution now starts from an empty registry. --- cmd/relay-server/config.go | 117 ++++++++++++++++++++++++++----------- cmd/relay-server/main.go | 5 ++ utils/cmd.go | 23 ++++++-- 3 files changed, 107 insertions(+), 38 deletions(-) diff --git a/cmd/relay-server/config.go b/cmd/relay-server/config.go index d573b187..c99fec5c 100644 --- a/cmd/relay-server/config.go +++ b/cmd/relay-server/config.go @@ -161,6 +161,15 @@ func ensGaslessFeature(cfg relayServerConfig) feature { f.Missing = "ACME_DNS_PROVIDER is empty; ENS gasless automation needs a DNS provider even when certificates are managed manually" return f } + // ENS gasless drives the same provider ACME does, so it cannot work when + // that provider cannot. Repeating only the "is it set" half of the check + // here would report this as enabled while acme is blocked, which is exactly + // the mismatch this report exists to surface. + if acme := acmeFeature(cfg); acme.State == stateBlocked { + f.State, f.By = stateBlocked, "ENS_GASLESS_ENABLED=true" + f.Missing = "the DNS provider it shares with ACME is blocked: " + acme.Missing + return f + } f.State, f.By = stateEnabled, "ENS_GASLESS_ENABLED=true" f.Detail = "DNSSEC and ENS TXT automation through " + cfg.ACMEDNSProvider return f @@ -349,6 +358,33 @@ func secretEnvName(name string) bool { return false } +// valueMarker distinguishes a key that something supplied from one running on +// its default, so a long list can be skimmed for what the operator actually set. +func valueMarker(entry utils.EnvVar) string { + if entry.SetBy == "" { + return "--" + } + return "OK" +} + +// valueSource names where the effective value came from. When an alias supplied +// it, the alias is named: "I set AWS_REGION, why is the value different?" is +// answered by seeing that AWS_DEFAULT_REGION was consulted first. +func valueSource(entry utils.EnvVar, supplied map[string]bool, envFile string) string { + if entry.SetBy == "" { + return fmt.Sprintf("default (%s)", defaultDisplay(entry.Default)) + } + + origin := "process environment" + if supplied[entry.SetBy] { + origin = envFile + } + if entry.SetBy != entry.Name { + return fmt.Sprintf("%s, via the alias %s", origin, entry.SetBy) + } + return origin +} + func displayValue(name, value string) string { if strings.TrimSpace(value) == "" { return "" @@ -364,45 +400,58 @@ func writeConfigReport(w io.Writer, cfg relayServerConfig, entries []envFileEntr fmt.Fprintf(w, "Portal relay configuration (%s)\n\n", source) - if len(entries) > 0 { - fmt.Fprintln(w, "Keys") - var unknown []envFileEntry - for _, entry := range entries { - switch { - case relay[entry.Name].Name != "": - known := relay[entry.Name] - fmt.Fprintf(w, " OK %-30s %-24s relay --%s\n", - entry.Name, displayValue(entry.Name, entry.Value), known.Flag) - writeWrapped(w, known.Usage) - if pinned, ok := pinnedByTopology[entry.Name]; ok && entry.Value != pinned.Value { - fmt.Fprintf(w, " WARNING: pinned to %s by the bundled topology; %s\n", - pinned.Value, pinned.Reason) - } - if note := alsoConsumedBy[entry.Name]; note != "" { - fmt.Fprintf(w, " also: %s\n", note) - } - case externalEnvVars[entry.Name].Owner != "": - external := externalEnvVars[entry.Name] - fmt.Fprintf(w, " OK %-30s %-24s %s\n", - entry.Name, displayValue(entry.Name, entry.Value), external.Owner) - writeWrapped(w, external.Usage) - default: - unknown = append(unknown, entry) - } + supplied := make(map[string]bool, len(entries)) + for _, entry := range entries { + supplied[entry.Name] = true + } + + // Every relay key is listed with the value the flag actually resolved to, + // not the text of whichever line happened to appear in the file. A key that + // an alias or a higher-priority name overrode would otherwise read as though + // it were in effect, which is the confusion this report exists to remove. + fmt.Fprintln(w, "Keys") + for _, entry := range utils.EnvVars() { + fmt.Fprintf(w, " %-4s %-30s %-24s relay --%s\n", + valueMarker(entry), entry.Name, displayValue(entry.Name, entry.Value), entry.Flag) + fmt.Fprintf(w, " source: %s\n", valueSource(entry, supplied, source)) + writeWrapped(w, entry.Usage) + if pinned, ok := pinnedByTopology[entry.Name]; ok && entry.Value != pinned.Value { + fmt.Fprintf(w, " WARNING: pinned to %s by the bundled topology; %s\n", + pinned.Value, pinned.Reason) + } + if note := alsoConsumedBy[entry.Name]; note != "" { + fmt.Fprintf(w, " also: %s\n", note) + } + } + + // Keys owned by another component are only shown when actually supplied: + // the relay cannot resolve them, so there is no effective value to report. + var unknown []envFileEntry + for _, entry := range entries { + if _, isRelay := relay[entry.Name]; isRelay { + continue } + external, isExternal := externalEnvVars[entry.Name] + if !isExternal { + unknown = append(unknown, entry) + continue + } + fmt.Fprintf(w, " OK %-30s %-24s %s\n", + entry.Name, displayValue(entry.Name, entry.Value), external.Owner) + writeWrapped(w, external.Usage) + } - if len(unknown) > 0 { - fmt.Fprintf(w, "\nUNKNOWN %d key(s) are not read by any component and are silently ignored:\n", len(unknown)) - for _, entry := range unknown { - if suggestion := nearestEnvName(entry.Name, relay); suggestion != "" { - fmt.Fprintf(w, " %-30s did you mean %s?\n", entry.Name, suggestion) - continue - } - fmt.Fprintf(w, " %-30s no equivalent key exists\n", entry.Name) + if len(unknown) > 0 { + fmt.Fprintf(w, "\nUNKNOWN %d key(s) are not read by any component and are silently ignored:\n", len(unknown)) + for _, entry := range unknown { + if suggestion := nearestEnvName(entry.Name, relay); suggestion != "" { + fmt.Fprintf(w, " %-30s did you mean %s?\n", entry.Name, suggestion) + continue } + fmt.Fprintf(w, " %-30s no equivalent key exists\n", entry.Name) } - fmt.Fprintln(w) } + fmt.Fprintln(w) fmt.Fprintln(w, "Features") for _, f := range evaluateFeatures(cfg) { diff --git a/cmd/relay-server/main.go b/cmd/relay-server/main.go index ea1de30a..7fe586bc 100644 --- a/cmd/relay-server/main.go +++ b/cmd/relay-server/main.go @@ -77,6 +77,11 @@ type relayServerConfig struct { // process environment. The config subcommand reuses it so that inspecting a // deployment and running it read the same definitions. func resolveRelayServerConfig(args []string) (relayServerConfig, error) { + // Registration records into a process-global registry, so start from empty: + // the config subcommand loads an env file and resolves again, and issues + // from an earlier pass must not fail the current one. + utils.ResetEnvRegistry() + cfg := relayServerConfig{} fs := utils.NewFlagSet("relay-server", printRootUsage) diff --git a/utils/cmd.go b/utils/cmd.go index 77a780a1..aec5a6aa 100644 --- a/utils/cmd.go +++ b/utils/cmd.go @@ -31,6 +31,10 @@ type EnvVar struct { Flag string Usage string Default string + // Value is what the flag actually resolved to. Reporting the raw text of an + // env file instead would show a value that a higher-priority name overrode, + // or an alias that was never consulted, as though it were in effect. + Value string // SetBy is the environment variable that supplied the value, or empty when // the default was used. It answers "I set that, why did nothing change?". SetBy string @@ -64,7 +68,17 @@ func EnvIssues() []EnvIssue { return slices.Clone(envIssues) } -func registerEnvVar(flagName, usage, defaultValue, setBy string, envNames []string) { +// ResetEnvRegistry clears both the registry and the recorded issues so a +// resolution pass reports only its own environment. Registering a flag twice +// already replaces its entry, but issues would otherwise accumulate across +// passes and a stale one could fail a configuration that no longer has it. +func ResetEnvRegistry() { + envVars = nil + envVarIndex = map[string]int{} + envIssues = nil +} + +func registerEnvVar(flagName, usage, defaultValue, value, setBy string, envNames []string) { names := make([]string, 0, len(envNames)) for _, envName := range envNames { if envName = strings.TrimSpace(envName); envName != "" { @@ -81,6 +95,7 @@ func registerEnvVar(flagName, usage, defaultValue, setBy string, envNames []stri Flag: flagName, Usage: usage, Default: defaultValue, + Value: value, SetBy: setBy, } // Registering the same flag twice (a re-parsed command, a test) replaces the @@ -208,7 +223,7 @@ func StringFlag(fs *flag.FlagSet, target *string, name, fallback, usage string) func StringFlagEnv(fs *flag.FlagSet, target *string, name, fallback, usage string, envNames ...string) { value, setBy := resolveStringEnv(fallback, envNames...) - registerEnvVar(name, usage, fallback, setBy, envNames) + registerEnvVar(name, usage, fallback, value, setBy, envNames) ensureFlagSet(fs).StringVar(target, name, value, flagUsage(usage, envNames...)) } @@ -218,13 +233,13 @@ func BoolFlag(fs *flag.FlagSet, target *bool, name string, fallback bool, usage func BoolFlagEnv(fs *flag.FlagSet, target *bool, name string, fallback bool, usage string, envNames ...string) { value, setBy := resolveBoolEnv(fallback, envNames...) - registerEnvVar(name, usage, strconv.FormatBool(fallback), setBy, envNames) + registerEnvVar(name, usage, strconv.FormatBool(fallback), strconv.FormatBool(value), setBy, envNames) ensureFlagSet(fs).BoolVar(target, name, value, flagUsage(usage, envNames...)) } func IntFlagEnv(fs *flag.FlagSet, target *int, name string, fallback int, parse IntEnvParser, usage string, envNames ...string) { value, setBy := resolveIntEnv(fallback, parse, envNames...) - registerEnvVar(name, usage, strconv.Itoa(fallback), setBy, envNames) + registerEnvVar(name, usage, strconv.Itoa(fallback), strconv.Itoa(value), setBy, envNames) ensureFlagSet(fs).IntVar(target, name, value, flagUsage(usage, envNames...)) } From cddbb34419ea9e31dc74666cd6adb673b4c61b41 Mon Sep 17 00:00:00 2001 From: Hee Sung Son Date: Tue, 11 Aug 2026 13:05:37 +0900 Subject: [PATCH 3/7] fix(relay-server): make the config report describe the deployment it checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cases where the report could validate a configuration different from the one that would actually run. `--env-file` is documented as inspecting a file instead of the process environment, but it only set the file's own keys. A relay variable absent from the file stayed inherited from the shell, and a higher-priority alias in the shell beat a value the file did supply — process AWS_REGION over file AWS_DEFAULT_REGION. Compose passes only the file, so the report could describe a different deployment than the one being checked. The file is now the whole environment for that pass: every name the deployment understands is cleared first, the file applied, and the previous environment restored afterwards. Inspecting the process environment, where isolation is not the intent, is unchanged. ACME was reported as enabled whenever the provider and credential looked valid, including when PORTAL_URL had a local-only host. acme.NewManager returns before it builds a DNS provider for a local base domain, so managed issuance never runs in that case and a development certificate is used instead. The report now treats a configured provider on a local host as blocked, which is what asking for automation that cannot start should look like. ens-gasless already defers to acmeFeature, so it inherits the state and the reason. Both are the mismatch this report exists to surface, so both are covered by tests that fail when either fix is reverted. --- cmd/relay-server/config.go | 86 +++++++++++++++-- cmd/relay-server/config_test.go | 165 ++++++++++++++++++++++++++++++++ 2 files changed, 245 insertions(+), 6 deletions(-) create mode 100644 cmd/relay-server/config_test.go diff --git a/cmd/relay-server/config.go b/cmd/relay-server/config.go index c99fec5c..7147e9ce 100644 --- a/cmd/relay-server/config.go +++ b/cmd/relay-server/config.go @@ -123,6 +123,18 @@ func acmeFeature(cfg relayServerConfig) feature { return f } + // acme.NewManager returns before it builds a DNS provider when the base + // domain is local-only, so managed issuance cannot run however well the + // provider is configured. Reporting it as enabled here would describe an + // automation that never starts. + if host := portalURLHost(cfg.PortalURL); host == "" || utils.IsLocalRelayHost(host) { + f.State, f.By = stateBlocked, "ACME_DNS_PROVIDER="+provider + f.Missing = fmt.Sprintf( + "PORTAL_URL host %q is local-only; managed issuance is skipped for local hosts and a development certificate is used instead", + host) + return f + } + required, supported := dnsProviderCredential[provider] if !supported { f.State, f.By = stateBlocked, "ACME_DNS_PROVIDER="+provider @@ -641,6 +653,70 @@ func writeCommentWrapped(w io.Writer, text string) { } } +// applyEnvFileInIsolation makes the file the whole environment for the pass +// that follows, and returns a function restoring what was there before. +// +// Setting only the file's own keys is not enough. A relay variable absent from +// the file would stay inherited from the shell, and a higher-priority alias in +// the shell would beat a value the file does supply — process AWS_REGION over +// file AWS_DEFAULT_REGION, for instance. Either way the report would describe a +// configuration different from the one Compose is going to deploy, which is the +// opposite of what checking a file is for. +func applyEnvFileInIsolation(entries []envFileEntry) (func(), error) { + // A first pass populates the registry, which is how the set of names the + // deployment understands is known at all. + if _, err := resolveRelayServerConfig(nil); err != nil { + return nil, err + } + + names := make([]string, 0, len(externalEnvVars)) + for _, entry := range utils.EnvVars() { + names = append(names, entry.Name) + names = append(names, entry.Aliases...) + } + for name := range externalEnvVars { + names = append(names, name) + } + for _, entry := range entries { + names = append(names, entry.Name) + } + + type saved struct { + value string + set bool + } + previous := make(map[string]saved, len(names)) + restore := func() { + for name, prior := range previous { + if prior.set { + _ = os.Setenv(name, prior.value) + continue + } + _ = os.Unsetenv(name) + } + } + + for _, name := range names { + if _, recorded := previous[name]; recorded { + continue + } + value, set := os.LookupEnv(name) + previous[name] = saved{value: value, set: set} + if err := os.Unsetenv(name); err != nil { + restore() + return nil, fmt.Errorf("isolate %s: %w", name, err) + } + } + + for _, entry := range entries { + if err := os.Setenv(entry.Name, entry.Value); err != nil { + restore() + return nil, fmt.Errorf("apply %s: %w", entry.Name, err) + } + } + return restore, nil +} + func runConfigCommand(args []string) error { var ( envFilePath string @@ -661,8 +737,6 @@ func runConfigCommand(args []string) error { return err } - // Load the file before registering flags: flag defaults resolve from the - // process environment, which is exactly how Compose delivers env_file. var entries []envFileEntry source := "process environment" if strings.TrimSpace(envFilePath) != "" { @@ -670,11 +744,11 @@ func runConfigCommand(args []string) error { if err != nil { return fmt.Errorf("read env file: %w", err) } - for _, entry := range loaded { - if err := os.Setenv(entry.Name, entry.Value); err != nil { - return fmt.Errorf("apply %s: %w", entry.Name, err) - } + restore, err := applyEnvFileInIsolation(loaded) + if err != nil { + return err } + defer restore() entries = loaded source = envFilePath } diff --git a/cmd/relay-server/config_test.go b/cmd/relay-server/config_test.go new file mode 100644 index 00000000..18ed777a --- /dev/null +++ b/cmd/relay-server/config_test.go @@ -0,0 +1,165 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// writeEnvFile writes lines to a temporary env file and returns its path. +func writeEnvFile(t *testing.T, lines ...string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "test.env") + body := strings.Join(lines, "\n") + "\n" + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatalf("write env file: %v", err) + } + return path +} + +// resolveWithEnvFile runs the same isolation the config subcommand performs and +// returns the resulting configuration. +func resolveWithEnvFile(t *testing.T, path string) relayServerConfig { + t.Helper() + entries, err := loadEnvFile(path) + if err != nil { + t.Fatalf("load env file: %v", err) + } + restore, err := applyEnvFileInIsolation(entries) + if err != nil { + t.Fatalf("isolate env file: %v", err) + } + defer restore() + + cfg, err := resolveRelayServerConfig(nil) + if err != nil { + t.Fatalf("resolve config: %v", err) + } + return cfg +} + +func featureByName(t *testing.T, cfg relayServerConfig, name string) feature { + t.Helper() + for _, f := range evaluateFeatures(cfg) { + if f.Name == name { + return f + } + } + t.Fatalf("feature %q not reported", name) + return feature{} +} + +// A variable absent from the env file must not leak in from the surrounding +// shell: Compose passes only the file, so a report that saw the shell would be +// describing a different deployment. +func TestEnvFileIsolationIgnoresInheritedValue(t *testing.T) { + t.Setenv("DISCOVERY", "true") + + cfg := resolveWithEnvFile(t, writeEnvFile(t, "PORTAL_URL=https://relay.example.com")) + + if cfg.DiscoveryEnabled { + t.Fatal("DISCOVERY was inherited from the process environment; the file did not set it") + } +} + +// A higher-priority alias in the shell must not beat a value the file supplies +// through a lower-priority name. +func TestEnvFileIsolationBeatsHigherPriorityAlias(t *testing.T) { + t.Setenv("AWS_REGION", "us-east-1") + + cfg := resolveWithEnvFile(t, writeEnvFile(t, "AWS_DEFAULT_REGION=ap-northeast-2")) + + if cfg.AWSRegion != "ap-northeast-2" { + t.Fatalf("AWS region = %q, want the file value ap-northeast-2", cfg.AWSRegion) + } +} + +func TestEnvFileIsolationRestoresEnvironment(t *testing.T) { + t.Setenv("DISCOVERY", "true") + if err := os.Unsetenv("BOOTSTRAPS"); err != nil { + t.Fatalf("unset BOOTSTRAPS: %v", err) + } + + entries, err := loadEnvFile(writeEnvFile(t, "BOOTSTRAPS=https://seed.example.com")) + if err != nil { + t.Fatalf("load env file: %v", err) + } + restore, err := applyEnvFileInIsolation(entries) + if err != nil { + t.Fatalf("isolate env file: %v", err) + } + restore() + + if got := os.Getenv("DISCOVERY"); got != "true" { + t.Fatalf("DISCOVERY = %q after restore, want true", got) + } + if _, set := os.LookupEnv("BOOTSTRAPS"); set { + t.Fatal("BOOTSTRAPS is set after restore; it was unset before") + } +} + +// acme.NewManager returns before building a DNS provider for a local-only base +// domain, so a configured provider still yields no managed issuance. +func TestACMEFeatureBlockedForLocalHost(t *testing.T) { + cfg := relayServerConfig{ + PortalURL: "https://localhost", + ACMEDNSProvider: "cloudflare", + CloudflareToken: "token", + } + + f := featureByName(t, cfg, "acme") + if f.State != stateBlocked { + t.Fatalf("acme state = %q, want %q", f.State, stateBlocked) + } + if !strings.Contains(f.Missing, "local-only") { + t.Fatalf("acme missing = %q, want it to name the local-only host", f.Missing) + } +} + +// ens-gasless drives the same provider, so it must inherit the blocked state +// rather than repeating half of the check. +func TestENSGaslessFollowsBlockedACME(t *testing.T) { + cfg := relayServerConfig{ + PortalURL: "https://localhost", + ACMEDNSProvider: "cloudflare", + CloudflareToken: "token", + ENSGaslessEnabled: true, + } + + f := featureByName(t, cfg, "ens-gasless") + if f.State != stateBlocked { + t.Fatalf("ens-gasless state = %q, want %q", f.State, stateBlocked) + } + if !strings.Contains(f.Missing, "local-only") { + t.Fatalf("ens-gasless missing = %q, want the ACME reason propagated", f.Missing) + } +} + +func TestACMEFeatureEnabledForPublicHost(t *testing.T) { + cfg := relayServerConfig{ + PortalURL: "https://relay.example.com", + ACMEDNSProvider: "cloudflare", + CloudflareToken: "token", + } + + f := featureByName(t, cfg, "acme") + if f.State != stateEnabled { + t.Fatalf("acme state = %q, want %q (missing: %s)", f.State, stateEnabled, f.Missing) + } +} + +func TestACMEFeatureBlockedWithoutCredential(t *testing.T) { + cfg := relayServerConfig{ + PortalURL: "https://relay.example.com", + ACMEDNSProvider: "cloudflare", + } + + f := featureByName(t, cfg, "acme") + if f.State != stateBlocked { + t.Fatalf("acme state = %q, want %q", f.State, stateBlocked) + } + if !strings.Contains(f.Missing, "CLOUDFLARE_TOKEN") { + t.Fatalf("acme missing = %q, want it to name the credential", f.Missing) + } +} From 3be85574ebe8abd6bea80ffc9715b00be2379434 Mon Sep 17 00:00:00 2001 From: Hee Sung Son Date: Tue, 11 Aug 2026 15:58:06 +0900 Subject: [PATCH 4/7] fix(relay-server): mask secret values in the invalid-value list The Keys section runs every value through displayValue, which prints for a name that looks like a credential. The Invalid values section printed the raw text instead. Nothing leaks today: an issue is only recorded when a boolean or integer fails to parse, and no credential is either. But the two lists are read the same way, and a report that masks a secret in one place and prints it in another is one numeric credential away from pasting it into a bug thread. --- cmd/relay-server/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/relay-server/config.go b/cmd/relay-server/config.go index 7147e9ce..c86d8a13 100644 --- a/cmd/relay-server/config.go +++ b/cmd/relay-server/config.go @@ -483,7 +483,7 @@ func writeConfigReport(w io.Writer, cfg relayServerConfig, entries []envFileEntr if issues := utils.EnvIssues(); len(issues) > 0 { fmt.Fprintln(w, "\nInvalid values") for _, issue := range issues { - fmt.Fprintf(w, " %s=%s %s\n", issue.Name, issue.Value, issue.Problem) + fmt.Fprintf(w, " %s=%s %s\n", issue.Name, displayValue(issue.Name, issue.Value), issue.Problem) } } } From e78c2322b4dfd50f1cf3b18b4cf8faeb0be59de6 Mon Sep 17 00:00:00 2001 From: Hee Sung Son Date: Wed, 12 Aug 2026 14:48:08 +0900 Subject: [PATCH 5/7] docs: pass -T when the env file arrives on stdin The documented check reads the file from stdin, but `docker compose run` asks for a TTY unless told not to. Older Compose versions then fail with "the input device is not a TTY" and the operator never sees the report -- which is the one command this feature exists to offer. Newer versions detect the redirect and work either way, so the omission survives a local test and only shows up on the server. --- .env.example | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.env.example b/.env.example index 3912da5c..8c4b3a91 100644 --- a/.env.example +++ b/.env.example @@ -7,7 +7,10 @@ # effective value and source of every key, names any key that nothing reads, and # says which features are off and what is missing: # -# docker compose run --rm portal config --env-file /dev/stdin < .env +# docker compose run --rm -T portal config --env-file /dev/stdin < .env +# +# -T because the file arrives on stdin. Without it Compose asks for a TTY and +# older versions fail with "the input device is not a TTY". # # API_PORT and SNI_PORT are deliberately absent. The bundled topology fixes them # at 4017 and 443 because the relay reaches its own API listener through its SNI From f16b6225e9a26048db9585605b0e898f6265d8fc Mon Sep 17 00:00:00 2001 From: Hee Sung Son Date: Tue, 18 Aug 2026 23:27:43 +0900 Subject: [PATCH 6/7] fix(relay-server): stop the report from answering a question it was not asked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from review, all cases where the report was confident about something it had not actually checked. `--env-file` was documented as showing what a deployment will run, but it resolves the file against the relay binary's defaults. Compose supplies its own first: a file carrying only PORTAL_URL and UDP_ENABLED=true reported `udp-transport blocked` while `docker compose up` would give it MIN_PORT=40000 and enable it. Reimplementing Compose's defaults here would build a second configuration engine, so the claim is narrowed instead. The accurate check needs no new code — running the subcommand inside the container lets Compose build the environment first: docker compose run --rm -T portal config That is now what .env.example, the configuration page and the command's own usage recommend, and a file-scoped report says in its header which of the two questions it answered. `discoveryFeature` inspected only the parsed hostname, so PORTAL_URL=http://relay.example.com reported enabled and portal.NewServer rejected the same value seconds later — the exact divergence this feature exists to remove. It now calls utils.NormalizeRelayURL and utils.NormalizeRelayURLs, the same normalization the server applies, rather than holding a second opinion about it. Bootstraps are counted after normalization for the same reason. loadEnvFile silently skipped any line without an `=`. `DISCOVERY true` vanished and discovery reported its default with nothing to explain why, which is the silent misconfiguration this command was written to expose. Malformed lines now fail with file:line. Also keys dnsProviderCredential by acme's exported Type* constants rather than repeating the provider names, so acme stays the one place that decides what is supported and this map only adds the credential each one needs. --- .env.example | 14 +++-- cmd/relay-server/config.go | 51 ++++++++++++++--- cmd/relay-server/config_test.go | 79 ++++++++++++++++++++++++++ cmd/relay-server/envcatalog.go | 19 +++++-- docs/src/routes/configuration/+page.md | 22 ++++++- 5 files changed, 165 insertions(+), 20 deletions(-) diff --git a/.env.example b/.env.example index 8c4b3a91..6cf471ca 100644 --- a/.env.example +++ b/.env.example @@ -3,14 +3,20 @@ # Copy to .env and edit. Every key here is read either by the relay binary or by # Docker Compose; nothing else reads this file. # -# Check a configured file before starting anything. The report gives the +# Check the configuration before starting anything. The report gives the # effective value and source of every key, names any key that nothing reads, and # says which features are off and what is missing: # -# docker compose run --rm -T portal config --env-file /dev/stdin < .env +# docker compose run --rm -T portal config # -# -T because the file arrives on stdin. Without it Compose asks for a TTY and -# older versions fail with "the input device is not a TTY". +# No --env-file: Compose has already built the container's environment from this +# file plus its own defaults, so the report describes what `docker compose up` +# will actually run. Passing --env-file instead reads the file against the relay +# binary's defaults, which differ -- MIN_PORT is 0 there and 40000 under Compose +# -- and would report features as blocked that the deployment enables. +# +# -T because `docker compose run` asks for a TTY otherwise, and older versions +# fail with "the input device is not a TTY". # # API_PORT and SNI_PORT are deliberately absent. The bundled topology fixes them # at 4017 and 443 because the relay reaches its own API listener through its SNI diff --git a/cmd/relay-server/config.go b/cmd/relay-server/config.go index c86d8a13..d9543118 100644 --- a/cmd/relay-server/config.go +++ b/cmd/relay-server/config.go @@ -100,6 +100,15 @@ func discoveryFeature(cfg relayServerConfig) feature { f.State, f.By = stateDisabled, "DISCOVERY=false" return f } + // The same normalization portal.NewServer applies, not a second opinion + // about it. Checking only the parsed hostname would report PORTAL_URL=http://… + // as enabled and then have the server reject it seconds later, which is + // exactly the divergence this report exists to remove. + if _, err := utils.NormalizeRelayURL(cfg.PortalURL); err != nil { + f.State, f.By = stateBlocked, "DISCOVERY=true" + f.Missing = fmt.Sprintf("PORTAL_URL is not usable as a relay URL: %v", err) + return f + } host := portalURLHost(cfg.PortalURL) if host == "" || utils.IsLocalRelayHost(host) { f.State, f.By = stateBlocked, "DISCOVERY=true" @@ -108,9 +117,15 @@ func discoveryFeature(cfg relayServerConfig) feature { host) return f } + bootstraps, err := utils.NormalizeRelayURLs(utils.SplitCSV(cfg.Bootstraps)...) + if err != nil { + f.State, f.By = stateBlocked, "DISCOVERY=true" + f.Missing = fmt.Sprintf("BOOTSTRAPS is not usable: %v", err) + return f + } f.State, f.By = stateEnabled, "DISCOVERY=true" f.Detail = fmt.Sprintf("host=%s bootstraps=%d wireguard_port=%d", - host, len(utils.SplitCSV(cfg.Bootstraps)), cfg.WireGuardPort) + host, len(bootstraps), cfg.WireGuardPort) return f } @@ -321,19 +336,25 @@ func loadEnvFile(path string) ([]envFileEntry, error) { var entries []envFileEntry scanner := bufio.NewScanner(file) + lineNo := 0 for scanner.Scan() { + lineNo++ line := strings.TrimSpace(scanner.Text()) if line == "" || strings.HasPrefix(line, "#") { continue } line = strings.TrimPrefix(line, "export ") + // A line that is neither blank, a comment, nor an assignment is a + // mistake, and skipping it would reproduce the silent misconfiguration + // this command exists to expose: `DISCOVERY true` would simply vanish + // and the feature would report its default with nothing to explain why. name, value, found := strings.Cut(line, "=") if !found { - continue + return nil, fmt.Errorf("%s:%d: not an assignment: %q", path, lineNo, line) } name = strings.TrimSpace(name) if name == "" { - continue + return nil, fmt.Errorf("%s:%d: assignment has no name: %q", path, lineNo, line) } // Compose does not expand values read from an env file, so neither do we. value = strings.TrimSpace(value) @@ -410,7 +431,18 @@ func displayValue(name, value string) string { func writeConfigReport(w io.Writer, cfg relayServerConfig, entries []envFileEntry, source string) { relay := knownEnvNames() - fmt.Fprintf(w, "Portal relay configuration (%s)\n\n", source) + fmt.Fprintf(w, "Portal relay configuration (%s)\n", source) + // Say what was inspected, because the two modes answer different questions + // and only one of them describes a Compose deployment. Reading a file in + // isolation applies relay defaults to every key the file omits, while + // Compose supplies its own first: a file with only PORTAL_URL reports + // MIN_PORT=0 here, and `docker compose up` would run it with 40000. + if len(entries) > 0 { + fmt.Fprint(w, "Keys absent from this file take relay defaults. A Compose deployment\n"+ + "supplies its own first; for that environment run the command inside the\n"+ + "container instead: docker compose run --rm -T portal config\n") + } + fmt.Fprintln(w) supplied := make(map[string]bool, len(entries)) for _, entry := range entries { @@ -723,7 +755,11 @@ func runConfigCommand(args []string) error { format string ) fs := utils.NewFlagSet("relay-server config", printConfigUsage) - utils.StringFlag(fs, &envFilePath, "env-file", "", "env file to inspect instead of the process environment") + utils.StringFlag(fs, &envFilePath, "env-file", "", + "read this file in place of the process environment, against relay defaults. "+ + "Compose supplies its own defaults on top of a file, so to see what a Compose "+ + "deployment will actually run, omit this flag and let Compose build the environment: "+ + "docker compose run --rm -T portal config") utils.StringFlag(fs, &format, "format", "text", "output format: text, env or names") if err := utils.ParseFlagSet(fs, args, printConfigUsage); err != nil { @@ -780,8 +816,9 @@ func printConfigUsage(w io.Writer) { "relay-server config [--env-file PATH] [--format text|env]", }, []string{ - "relay-server config", - "relay-server config --env-file .env", + "docker compose run --rm -T portal config # what Compose will run", + "relay-server config # this process environment", + "relay-server config --env-file .env # one file, against relay defaults", "relay-server config --format env > env.reference", }, ) diff --git a/cmd/relay-server/config_test.go b/cmd/relay-server/config_test.go index 18ed777a..aaa7885f 100644 --- a/cmd/relay-server/config_test.go +++ b/cmd/relay-server/config_test.go @@ -163,3 +163,82 @@ func TestACMEFeatureBlockedWithoutCredential(t *testing.T) { t.Fatalf("acme missing = %q, want it to name the credential", f.Missing) } } + +// A line that is neither blank, a comment, nor an assignment is a typo, and +// dropping it would recreate the silent misconfiguration this command exists to +// expose: the feature would report its default with nothing to explain why. +func TestLoadEnvFileRejectsMalformedLines(t *testing.T) { + for name, line := range map[string]string{ + "missing separator": "DISCOVERY true", + "empty name": "=true", + } { + t.Run(name, func(t *testing.T) { + path := writeEnvFile(t, "PORTAL_URL=https://relay.example.com", line) + + _, err := loadEnvFile(path) + if err == nil { + t.Fatalf("%q was accepted", line) + } + if !strings.Contains(err.Error(), ":2:") { + t.Fatalf("error does not point at the line: %v", err) + } + }) + } +} + +func TestLoadEnvFileKeepsCommentsAndBlanks(t *testing.T) { + path := writeEnvFile(t, "# a comment", "", " ", "export PORTAL_URL=https://relay.example.com") + + entries, err := loadEnvFile(path) + if err != nil { + t.Fatalf("load env file: %v", err) + } + if len(entries) != 1 || entries[0].Name != "PORTAL_URL" { + t.Fatalf("entries = %v, want only PORTAL_URL", entries) + } +} + +// The report must not claim a feature works when the server will reject the +// same value moments later. portal.NewServer normalizes PORTAL_URL through +// utils.NormalizeRelayURL, which requires https. +func TestDiscoveryBlockedForNonHTTPSPortalURL(t *testing.T) { + path := writeEnvFile(t, "DISCOVERY=true", "PORTAL_URL=http://relay.example.com") + cfg := resolveWithEnvFile(t, path) + + f := discoveryFeature(cfg) + if f.State != stateBlocked { + t.Fatalf("discovery state = %q, want blocked for a non-https PORTAL_URL", f.State) + } + if !strings.Contains(f.Missing, "https") { + t.Fatalf("missing = %q, want it to name the https requirement", f.Missing) + } +} + +func TestDiscoveryBlockedForUnusableBootstraps(t *testing.T) { + path := writeEnvFile(t, + "DISCOVERY=true", + "PORTAL_URL=https://relay.example.com", + "BOOTSTRAPS=http://peer.example.com") + cfg := resolveWithEnvFile(t, path) + + f := discoveryFeature(cfg) + if f.State != stateBlocked { + t.Fatalf("discovery state = %q, want blocked for an unusable BOOTSTRAPS", f.State) + } +} + +func TestDiscoveryEnabledCountsNormalizedBootstraps(t *testing.T) { + path := writeEnvFile(t, + "DISCOVERY=true", + "PORTAL_URL=https://relay.example.com", + "BOOTSTRAPS=https://a.example.com,https://b.example.com") + cfg := resolveWithEnvFile(t, path) + + f := discoveryFeature(cfg) + if f.State != stateEnabled { + t.Fatalf("discovery state = %q, want enabled", f.State) + } + if !strings.Contains(f.Detail, "bootstraps=2") { + t.Fatalf("detail = %q, want bootstraps=2", f.Detail) + } +} diff --git a/cmd/relay-server/envcatalog.go b/cmd/relay-server/envcatalog.go index a26d59ab..7733e220 100644 --- a/cmd/relay-server/envcatalog.go +++ b/cmd/relay-server/envcatalog.go @@ -1,5 +1,7 @@ package main +import "github.com/gosuda/portal-tunnel/v2/portal/acme" + // The deployment .env is shared by the relay and by Docker Compose itself. // Checking a key against the relay's own flags alone would report the // Compose-level ones as unknown, so the keys owned elsewhere are catalogued @@ -64,11 +66,16 @@ var pinnedByTopology = map[string]struct { // credential it requires. Providers whose credentials come from an ambient // chain (an instance role, application default credentials) map to an empty // list because there is nothing to require. +// +// The keys are acme's own exported constants rather than repeated strings, so +// a provider added there cannot silently go unreported here: acme.NewDNSProvider +// decides what is supported, and this map only adds the credential each one +// needs, which is knowledge the report owns. var dnsProviderCredential = map[string][]string{ - "cloudflare": {"CLOUDFLARE_TOKEN"}, - "hetzner": {"HETZNER_API_TOKEN"}, - "njalla": {"NJALLA_TOKEN"}, - "vultr": {"VULTR_API_KEY"}, - "route53": nil, - "gcloud": nil, + acme.TypeCloudflare: {"CLOUDFLARE_TOKEN"}, + acme.TypeHetzner: {"HETZNER_API_TOKEN"}, + acme.TypeNjalla: {"NJALLA_TOKEN"}, + acme.TypeVultr: {"VULTR_API_KEY"}, + acme.TypeRoute53: nil, + acme.TypeGCloud: nil, } diff --git a/docs/src/routes/configuration/+page.md b/docs/src/routes/configuration/+page.md index a4268ca8..633b4ea1 100644 --- a/docs/src/routes/configuration/+page.md +++ b/docs/src/routes/configuration/+page.md @@ -13,13 +13,29 @@ This page describes what each variable means. To see what a specific deployment is actually doing, ask the binary rather than reading a table: ```bash -relay-server config --env-file .env +docker compose run --rm -T portal config ``` It prints every key with its effective value and where that value came from, names any key nothing reads, and reports which features are off and what is -missing. `relay-server config --format env` regenerates the full list from the -flag definitions, and `make check-env-example` fails when this page or +missing. + +Run it **inside the container, without `--env-file`**. Compose has already +combined `.env` with the defaults declared in `docker-compose.yml`, so the +report then describes the environment `docker compose up` will actually +provide. `--env-file` reads a file on its own, against the relay binary's +defaults — `MIN_PORT` is `0` there and `40000` under Compose — so a file that +sets only `PORTAL_URL` and `UDP_ENABLED=true` is reported as +`udp-transport blocked` although the deployment would enable it. Use it to +inspect a file in isolation, not to predict a deployment: + +```bash +relay-server config # this process environment +relay-server config --env-file .env # one file, against relay defaults +``` + +`relay-server config --format env` regenerates the full list from the flag +definitions, and `make check-env-example` fails when this page or `.env.example` stops mentioning a key. ## Relay Server Environment Variables From 8586c426878bdcab06fcc9d940439d697e9e8e27 Mon Sep 17 00:00:00 2001 From: Hee Sung Son Date: Thu, 20 Aug 2026 18:45:27 +0900 Subject: [PATCH 7/7] fix(relay-server): read CLOUDFLARE_TOKEN again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The flag that binds CLOUDFLARE_TOKEN to relayServerConfig.CloudflareToken is gone from main. The struct field is still there and is still passed to acme.Config, so nothing fails to compile — it is simply always empty, and a relay configured with ACME_DNS_PROVIDER=cloudflare starts, reports itself configured, and then fails DNS-01 with "cloudflare token is required". The token appears in .env.example, docker-compose.yml and three documentation pages; it appears in no Go file. Every other provider credential is still wired, so this is Cloudflare alone. This branch's own report is what surfaced it, from two directions at once: UNKNOWN 1 key(s) are not read by any component and are silently ignored: CLOUDFLARE_TOKEN did you mean HCLOUD_TOKEN? ! acme blocked ACME_DNS_PROVIDER=cloudflare missing: CLOUDFLARE_TOKEN is empty Kept as its own commit rather than folded into the merge, so it can be taken separately or ahead of the rest. --- cmd/relay-server/main.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/relay-server/main.go b/cmd/relay-server/main.go index bb1f3834..7894cd8b 100644 --- a/cmd/relay-server/main.go +++ b/cmd/relay-server/main.go @@ -114,6 +114,7 @@ func resolveRelayServerConfig(args []string) (relayServerConfig, error) { utils.StringFlagEnv(fs, &cfg.ACMEDNSProvider, "acme-dns-provider", "", "DNS provider for managed DNS-01/A-record sync, ECH HTTPS records, and ENS gasless DNSSEC/TXT automation (embedded|cloudflare|gcloud|hetzner|njalla|route53|vultr); defaults to embedded when unset", "ACME_DNS_PROVIDER") utils.BoolFlagEnv(fs, &cfg.ENSGaslessEnabled, "ens-gasless-enabled", false, "enable ENS gasless DNS import automation for the managed DNS zone and lease hostnames", "ENS_GASLESS_ENABLED") utils.IntFlagEnv(fs, &cfg.EmbeddedDNSPort, "embedded-dns-port", 53, utils.ParsePortNumber, "listen port for the embedded authoritative DNS server (the default DNS provider); requires a one-time NS delegation of the base domain and open 53/tcp+udp", "EMBEDDED_DNS_PORT") + utils.StringFlagEnv(fs, &cfg.CloudflareToken, "cloudflare-token", "", "Cloudflare DNS API token for DNS automation (required when acme-dns-provider=cloudflare)", "CLOUDFLARE_TOKEN") utils.StringFlagEnv(fs, &cfg.GCPProjectID, "gcp-project-id", "", "Google Cloud project id for Cloud DNS automation; auto-detected from ADC or GCE metadata when omitted", "GCP_PROJECT_ID", "GOOGLE_CLOUD_PROJECT", "GCLOUD_PROJECT", "GCE_PROJECT") utils.StringFlagEnv(fs, &cfg.GCPManagedZone, "gcp-managed-zone", "", "explicit Google Cloud DNS managed zone name or numeric ID override", "GCP_MANAGED_ZONE", "GCP_ZONE", "GCE_ZONE_ID") utils.StringFlagEnv(fs, &cfg.HetznerAPIToken, "hetzner-api-token", "", "Hetzner Cloud API token for DNS automation (required when acme-dns-provider=hetzner)", "HETZNER_API_TOKEN", "HCLOUD_TOKEN")