amadan
provision: close three door-validation gaps in ValidateShape
Fix three findings from coordinator review:
1. Addr port validation: net.SplitHostPort accepts garbage ports
(e.g. "127.0.0.1:abc"). Parse port as uint16 and reject if not
in range 1..65535, refusing materialization of routes that never
connect (the -backing unti incident class).
2. Environment validation: Environment field had no door check but
reaches bucket key construction in appconfig.EnvironmentKey.
Validate using appconfig.ValidEnvironmentName to reject invalid
environment names before they bind silently to wrong bundles.
3. Service kind requires upstream: A service record without Addr
contradicts the design ("the platform only proxies to a service"),
so refuse it at the door. Also validates DerivesPaths correctly
for services (always have Addr, never socket-backed).
Add three test cases: bad port, bad environment name, and service
with no addr.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 files changed,
+20
−0
internal/provision/provision.go+16 −0internal/provision/provision_test.go+4 −0
diff --git a/internal/provision/provision.go b/internal/provision/provision.go| index b622b55bb..2e9f97dbe 100644 |
| --- a/internal/provision/provision.go |
| +++ b/internal/provision/provision.go |
| @@ -16,9 +16,11 @@ import ( |
| "path/filepath" |
| "regexp" |
| "sort" |
| + "strconv" |
| "strings" |
| "time" |
| + "github.com/carlosframework/platform/internal/appconfig" |
| "github.com/carlosframework/platform/internal/registry" |
| "github.com/carlosframework/platform/internal/release" |
| ) |
| @@ -231,10 +233,21 @@ func ValidateShape(rec Record) error { |
| if err != nil || h == "" || p == "" { |
| return fmt.Errorf("addr %q: want a TCP upstream as host:port (e.g. 127.0.0.1:8090)", rec.Addr) |
| } |
| + // Parse port as integer and validate range 1..65535. net.SplitHostPort |
| + // accepts garbage ports (e.g. "127.0.0.1:abc") — we need to refuse them |
| + // here at the door, not discover them months later as a route that never |
| + // connects (the -backing unti class of incident). |
| + portNum, err := strconv.ParseUint(p, 10, 16) |
| + if err != nil || portNum == 0 { |
| + return fmt.Errorf("addr %q: port must be an integer in the range 1..65535", rec.Addr) |
| + } |
| } |
| if rec.Unit != "" && !registry.ValidUnitName(rec.Unit) { |
| return fmt.Errorf("unit %q: want a systemd service unit name (e.g. console.service)", rec.Unit) |
| } |
| + if rec.Environment != "" && !appconfig.ValidEnvironmentName(rec.Environment) { |
| + return fmt.Errorf("environment %q: want [a-z0-9-]{1,32}", rec.Environment) |
| + } |
| if rec.KindOrDefault() == "static" { |
| if rec.Addr != "" { |
| return errors.New("kind static with addr: a static route serves a file tree and proxies to nothing — drop one of the two") |
| @@ -246,6 +259,9 @@ func ValidateShape(rec Record) error { |
| return errors.New("kind static with hibernate: a static route has no process to wake, so arming the activator for it would wedge the host — drop hibernate") |
| } |
| } |
| + if rec.KindOrDefault() == "service" && rec.Addr == "" { |
| + return errors.New("kind service with no addr: the platform only proxies to a service, so the record must name the upstream it proxies to") |
| + } |
| return nil |
| } |
diff --git a/internal/provision/provision_test.go b/internal/provision/provision_test.go| index c96287e1e..d7b8b4f08 100644 |
| --- a/internal/provision/provision_test.go |
| +++ b/internal/provision/provision_test.go |
| @@ -468,6 +468,7 @@ func TestValidateShape(t *testing.T) { |
| {"legacy instance", base}, |
| {"addr tenant", with(func(r *Record) { r.Addr = "127.0.0.1:8090" })}, |
| {"unit-owned tenant", with(func(r *Record) { r.Addr = "127.0.0.1:8090"; r.Unit = "console.service" })}, |
| + {"service with addr", with(func(r *Record) { r.Kind = "service"; r.Addr = "127.0.0.1:8090" })}, |
| {"static", with(func(r *Record) { r.Kind = "static" })}, |
| {"explicitly non-hibernating instance", with(func(r *Record) { r.Hibernate = new(bool) })}, |
| } |
| @@ -483,7 +484,10 @@ func TestValidateShape(t *testing.T) { |
| {"bogus kind", with(func(r *Record) { r.Kind = "unti" })}, |
| {"status kind", with(func(r *Record) { r.Kind = "status" })}, |
| {"addr without port", with(func(r *Record) { r.Addr = "127.0.0.1" })}, |
| + {"addr with garbage port", with(func(r *Record) { r.Addr = "127.0.0.1:abc" })}, |
| {"unit typo", with(func(r *Record) { r.Unit = "console" })}, |
| + {"bad environment name", with(func(r *Record) { r.Environment = "Demo" })}, |
| + {"service with no addr", with(func(r *Record) { r.Kind = "service" })}, |
| {"static with addr", with(func(r *Record) { r.Kind = "static"; r.Addr = "127.0.0.1:8090" })}, |
| {"static with unit", with(func(r *Record) { r.Kind = "static"; r.Unit = "console.service" })}, |
| {"static hibernating", with(func(r *Record) { r.Kind = "static"; r.Hibernate = &yes })}, |