fix(brokerconfig): validate derived route authority
- commit
- 0cfd75aa807f78f3bd28a70cca6d3406e74a8dbe
- author
- brice · · 2026-09-23 14:42 UTC
- committer
- brice · · 2026-09-23 14:49 UTC
- parent
- 986c5fc
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +72 -3
M internal/adapters/brokerconfig/config.go +12 -1
| @@ -567,7 +567,18 @@ func (c *Config) Resolver() *Resolver { | |||
| 567 | 567 | if c == nil { | |
| 568 | 568 | return &Resolver{byEndpoint: map[string]Registration{}} | |
| 569 | 569 | } | |
| 570 | - | return &Resolver{byEndpoint: c.byEndpoint, local: c.local} | |
| 570 | + | byEndpoint := make(map[string]Registration, len(c.byEndpoint)) | |
| 571 | + | for endpoint, registration := range c.byEndpoint { | |
| 572 | + | registration.Route.argv = append([]string(nil), registration.Route.argv...) | |
| 573 | + | byEndpoint[endpoint] = registration | |
| 574 | + | } | |
| 575 | + | var local *LocalBinding | |
| 576 | + | if c.local != nil { | |
| 577 | + | copy := *c.local | |
| 578 | + | copy.Route.argv = append([]string(nil), copy.Route.argv...) | |
| 579 | + | local = © | |
| 580 | + | } | |
| 581 | + | return &Resolver{byEndpoint: byEndpoint, local: local} | |
| 571 | 582 | } | |
| 572 | 583 | ||
| 573 | 584 | // configDocument is the strict on-disk shape. Unknown fields are refused by the | |
| @@ -567,7 +567,18 @@ func (c *Config) Resolver() *Resolver { | |||||
| 567 | if c == nil { | 567 | if c == nil { | ||
| 568 | return &Resolver{byEndpoint: map[string]Registration{}} | 568 | return &Resolver{byEndpoint: map[string]Registration{}} | ||
| 569 | } | 569 | } | ||
| 570 | - | return &Resolver{byEndpoint: c.byEndpoint, local: c.local} | 570 | + | byEndpoint := make(map[string]Registration, len(c.byEndpoint)) |
| 571 | + | for endpoint, registration := range c.byEndpoint { | |||
| 572 | + | registration.Route.argv = append([]string(nil), registration.Route.argv...) | |||
| 573 | + | byEndpoint[endpoint] = registration | |||
| 574 | + | } | |||
| 575 | + | var local *LocalBinding | |||
| 576 | + | if c.local != nil { | |||
| 577 | + | copy := *c.local | |||
| 578 | + | copy.Route.argv = append([]string(nil), copy.Route.argv...) | |||
| 579 | + | local = © | |||
| 580 | + | } | |||
| 581 | + | return &Resolver{byEndpoint: byEndpoint, local: local} | |||
| 571 | } | 582 | } | ||
| 572 | 583 | ||||
| 573 | // configDocument is the strict on-disk shape. Unknown fields are refused by the | 584 | // configDocument is the strict on-disk shape. Unknown fields are refused by the | ||
M internal/adapters/brokerconfig/config_test.go +44 -0
| @@ -704,3 +704,47 @@ func TestLoadWarmTransports(t *testing.T) { | |||
| 704 | 704 | require.Equal(t, DefaultWarmTransports, none.WarmTransports()) | |
| 705 | 705 | require.Zero(t, none.WarmIdleTimeout()) | |
| 706 | 706 | } | |
| 707 | + | ||
| 708 | + | func TestRouteValidateDerivedAuthority(t *testing.T) { | |
| 709 | + | unix, err := newUnixRoute("/tmp/vev-brokerconfig-test/mux.sock") | |
| 710 | + | require.NoError(t, err) | |
| 711 | + | ssh, err := newSSHRoute(RouteSSHStdio, "host", []string{"vev"}, TrustInputs{}) | |
| 712 | + | require.NoError(t, err) | |
| 713 | + | tests := []struct { | |
| 714 | + | name string | |
| 715 | + | route Route | |
| 716 | + | valid bool | |
| 717 | + | }{ | |
| 718 | + | {"unix", unix, true}, | |
| 719 | + | {"ssh", ssh, true}, | |
| 720 | + | {"missing address", Route{kind: RouteUnix, path: unix.path, local: true}, false}, | |
| 721 | + | {"wrong address", Route{kind: RouteUnix, path: unix.path, local: true, address: ssh.address}, false}, | |
| 722 | + | {"unix not local", Route{kind: RouteUnix, path: unix.path, address: unix.address}, false}, | |
| 723 | + | {"ssh local", Route{kind: ssh.kind, target: ssh.target, argv: ssh.argv, address: ssh.address, local: true}, false}, | |
| 724 | + | } | |
| 725 | + | for _, tt := range tests { | |
| 726 | + | t.Run(tt.name, func(t *testing.T) { | |
| 727 | + | err := tt.route.Validate() | |
| 728 | + | if tt.valid { | |
| 729 | + | require.NoError(t, err) | |
| 730 | + | } else { | |
| 731 | + | require.Error(t, err) | |
| 732 | + | } | |
| 733 | + | }) | |
| 734 | + | } | |
| 735 | + | } | |
| 736 | + | ||
| 737 | + | func TestResolverDoesNotAliasConfiguration(t *testing.T) { | |
| 738 | + | config, err := loadDocument(t, documentWithLocal(t, localTestDocument()), nil) | |
| 739 | + | require.NoError(t, err) | |
| 740 | + | resolver := config.Resolver() | |
| 741 | + | original := config.Resolver() | |
| 742 | + | registration := resolver.byEndpoint[testEndpoint] | |
| 743 | + | registration.Route.argv = []string{"changed"} | |
| 744 | + | resolver.byEndpoint[testEndpoint] = registration | |
| 745 | + | delete(resolver.byEndpoint, testEndpoint) | |
| 746 | + | require.Equal(t, original.byEndpoint[testEndpoint], config.Resolver().byEndpoint[testEndpoint]) | |
| 747 | + | require.NotNil(t, resolver.local) | |
| 748 | + | resolver.local.Identity = "changed" | |
| 749 | + | require.Equal(t, original.local.Identity, config.Resolver().local.Identity) | |
| 750 | + | } | |
| @@ -704,3 +704,47 @@ func TestLoadWarmTransports(t *testing.T) { | |||||
| 704 | require.Equal(t, DefaultWarmTransports, none.WarmTransports()) | 704 | require.Equal(t, DefaultWarmTransports, none.WarmTransports()) | ||
| 705 | require.Zero(t, none.WarmIdleTimeout()) | 705 | require.Zero(t, none.WarmIdleTimeout()) | ||
| 706 | } | 706 | } | ||
| 707 | + | ||||
| 708 | + | func TestRouteValidateDerivedAuthority(t *testing.T) { | |||
| 709 | + | unix, err := newUnixRoute("/tmp/vev-brokerconfig-test/mux.sock") | |||
| 710 | + | require.NoError(t, err) | |||
| 711 | + | ssh, err := newSSHRoute(RouteSSHStdio, "host", []string{"vev"}, TrustInputs{}) | |||
| 712 | + | require.NoError(t, err) | |||
| 713 | + | tests := []struct { | |||
| 714 | + | name string | |||
| 715 | + | route Route | |||
| 716 | + | valid bool | |||
| 717 | + | }{ | |||
| 718 | + | {"unix", unix, true}, | |||
| 719 | + | {"ssh", ssh, true}, | |||
| 720 | + | {"missing address", Route{kind: RouteUnix, path: unix.path, local: true}, false}, | |||
| 721 | + | {"wrong address", Route{kind: RouteUnix, path: unix.path, local: true, address: ssh.address}, false}, | |||
| 722 | + | {"unix not local", Route{kind: RouteUnix, path: unix.path, address: unix.address}, false}, | |||
| 723 | + | {"ssh local", Route{kind: ssh.kind, target: ssh.target, argv: ssh.argv, address: ssh.address, local: true}, false}, | |||
| 724 | + | } | |||
| 725 | + | for _, tt := range tests { | |||
| 726 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 727 | + | err := tt.route.Validate() | |||
| 728 | + | if tt.valid { | |||
| 729 | + | require.NoError(t, err) | |||
| 730 | + | } else { | |||
| 731 | + | require.Error(t, err) | |||
| 732 | + | } | |||
| 733 | + | }) | |||
| 734 | + | } | |||
| 735 | + | } | |||
| 736 | + | ||||
| 737 | + | func TestResolverDoesNotAliasConfiguration(t *testing.T) { | |||
| 738 | + | config, err := loadDocument(t, documentWithLocal(t, localTestDocument()), nil) | |||
| 739 | + | require.NoError(t, err) | |||
| 740 | + | resolver := config.Resolver() | |||
| 741 | + | original := config.Resolver() | |||
| 742 | + | registration := resolver.byEndpoint[testEndpoint] | |||
| 743 | + | registration.Route.argv = []string{"changed"} | |||
| 744 | + | resolver.byEndpoint[testEndpoint] = registration | |||
| 745 | + | delete(resolver.byEndpoint, testEndpoint) | |||
| 746 | + | require.Equal(t, original.byEndpoint[testEndpoint], config.Resolver().byEndpoint[testEndpoint]) | |||
| 747 | + | require.NotNil(t, resolver.local) | |||
| 748 | + | resolver.local.Identity = "changed" | |||
| 749 | + | require.Equal(t, original.local.Identity, config.Resolver().local.Identity) | |||
| 750 | + | } | |||
M internal/adapters/brokerconfig/route.go +16 -2
| @@ -131,6 +131,20 @@ func (r Route) IsLocal() bool { return r.local } | |||
| 131 | 131 | // Validate re-checks one already-parsed route. Parsed routes are validated | |
| 132 | 132 | // once, so this is the seam a caller uses to fence a zero or hand-built value. | |
| 133 | 133 | func (r Route) Validate() error { | |
| 134 | + | if err := r.validateFields(); err != nil { | |
| 135 | + | return err | |
| 136 | + | } | |
| 137 | + | if r.address != r.digest() { | |
| 138 | + | return errors.New("route address does not match its contents") | |
| 139 | + | } | |
| 140 | + | if r.local != (r.kind == RouteUnix) { | |
| 141 | + | return errors.New("route locality does not match its kind") | |
| 142 | + | } | |
| 143 | + | return nil | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | // validateFields runs before constructors derive the opaque address. | |
| 147 | + | func (r Route) validateFields() error { | |
| 134 | 148 | switch r.kind { | |
| 135 | 149 | case RouteUnix: | |
| 136 | 150 | if err := validateRoute(r.path); err != nil { | |
| @@ -332,7 +346,7 @@ func RouteFromSpec(spec ports.BrokerRouteSpec) (Route, error) { | |||
| 332 | 346 | ||
| 333 | 347 | func newUnixRoute(path string) (Route, error) { | |
| 334 | 348 | route := Route{kind: RouteUnix, path: path, local: true} | |
| 335 | - | if err := route.Validate(); err != nil { | |
| 349 | + | if err := route.validateFields(); err != nil { | |
| 336 | 350 | return Route{}, err | |
| 337 | 351 | } | |
| 338 | 352 | route.address = route.digest() | |
| @@ -346,7 +360,7 @@ func newSSHRoute(kind RouteKind, target string, argv []string, trust TrustInputs | |||
| 346 | 360 | argv: append([]string(nil), argv...), | |
| 347 | 361 | trust: trust, | |
| 348 | 362 | } | |
| 349 | - | if err := route.Validate(); err != nil { | |
| 363 | + | if err := route.validateFields(); err != nil { | |
| 350 | 364 | return Route{}, err | |
| 351 | 365 | } | |
| 352 | 366 | route.address = route.digest() | |
| @@ -131,6 +131,20 @@ func (r Route) IsLocal() bool { return r.local } | |||||
| 131 | // Validate re-checks one already-parsed route. Parsed routes are validated | 131 | // Validate re-checks one already-parsed route. Parsed routes are validated | ||
| 132 | // once, so this is the seam a caller uses to fence a zero or hand-built value. | 132 | // once, so this is the seam a caller uses to fence a zero or hand-built value. | ||
| 133 | func (r Route) Validate() error { | 133 | func (r Route) Validate() error { | ||
| 134 | + | if err := r.validateFields(); err != nil { | |||
| 135 | + | return err | |||
| 136 | + | } | |||
| 137 | + | if r.address != r.digest() { | |||
| 138 | + | return errors.New("route address does not match its contents") | |||
| 139 | + | } | |||
| 140 | + | if r.local != (r.kind == RouteUnix) { | |||
| 141 | + | return errors.New("route locality does not match its kind") | |||
| 142 | + | } | |||
| 143 | + | return nil | |||
| 144 | + | } | |||
| 145 | + | ||||
| 146 | + | // validateFields runs before constructors derive the opaque address. | |||
| 147 | + | func (r Route) validateFields() error { | |||
| 134 | switch r.kind { | 148 | switch r.kind { | ||
| 135 | case RouteUnix: | 149 | case RouteUnix: | ||
| 136 | if err := validateRoute(r.path); err != nil { | 150 | if err := validateRoute(r.path); err != nil { | ||
| @@ -332,7 +346,7 @@ func RouteFromSpec(spec ports.BrokerRouteSpec) (Route, error) { | |||||
| 332 | 346 | ||||
| 333 | func newUnixRoute(path string) (Route, error) { | 347 | func newUnixRoute(path string) (Route, error) { | ||
| 334 | route := Route{kind: RouteUnix, path: path, local: true} | 348 | route := Route{kind: RouteUnix, path: path, local: true} | ||
| 335 | - | if err := route.Validate(); err != nil { | 349 | + | if err := route.validateFields(); err != nil { |
| 336 | return Route{}, err | 350 | return Route{}, err | ||
| 337 | } | 351 | } | ||
| 338 | route.address = route.digest() | 352 | route.address = route.digest() | ||
| @@ -346,7 +360,7 @@ func newSSHRoute(kind RouteKind, target string, argv []string, trust TrustInputs | |||||
| 346 | argv: append([]string(nil), argv...), | 360 | argv: append([]string(nil), argv...), | ||
| 347 | trust: trust, | 361 | trust: trust, | ||
| 348 | } | 362 | } | ||
| 349 | - | if err := route.Validate(); err != nil { | 363 | + | if err := route.validateFields(); err != nil { |
| 350 | return Route{}, err | 364 | return Route{}, err | ||
| 351 | } | 365 | } | ||
| 352 | route.address = route.digest() | 366 | route.address = route.digest() | ||