amadan
registry: validate -tunnel names, add SetTunnel unknown-host test
Review follow-up: -channel/-environment both validate shape before the
value reaches the registry; -tunnel had no such check, so a value like
"../../x" or one with spaces landed verbatim in the column. Adds
registry.ValidTunnelName ([a-z0-9-]{1,32}, same shape as
appconfig.ValidEnvironmentName), wired into cli.go's runRoute the same
way -environment's check is — after the "-" clear-sentinel test. Also
adds TestSetTunnelUnknownHostErrors, the missing unmatched-host case
for SetTunnel.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 files changed,
+99
−14
cli.go+24 −14internal/registry/registry.go+21 −0internal/registry/registry_test.go+54 −0
diff --git a/cli.go b/cli.go| index fdaa915f9..a3f268290 100644 |
| --- a/cli.go |
| +++ b/cli.go |
| @@ -219,15 +219,18 @@ func runInstances(args []string) { |
| // |
| // A third, independent axis, -tunnel, names the reverse channel a route is |
| // bound to at the edge (registry.Route.Tunnel — backing for later work in |
| -// the remote-host-agent feature, issue #53). It is simpler still: no bucket |
| -// to check existence against (a tunnel only becomes real once a host-agent |
| -// dials in under that name), and -host only — there is no bulk sibling to |
| -// SetTunnel, so -app + -tunnel is refused outright rather than silently |
| -// doing nothing. Same "-tunnel -" clear sentinel and the same |
| -// platformCanaryOwnedRow refusal as -channel/-environment, for the same |
| -// structural reason. It writes through its own SetTunnel call, independent |
| -// of the -channel/-environment switch below — with only ever one candidate, |
| -// there is no combined transaction for it to join. |
| +// the remote-host-agent feature, issue #53). It is simpler than -environment |
| +// in one way and matches it in another: shape is checked the same way |
| +// (registry.ValidTunnelName, same [a-z0-9-]{1,32} pattern as |
| +// ValidEnvironmentName), but there is no bucket to check EXISTENCE against |
| +// (a tunnel only becomes real once a host-agent dials in under that name) — |
| +// and it is -host only, since there is no bulk sibling to SetTunnel, so -app |
| +// + -tunnel is refused outright rather than silently doing nothing. Same |
| +// "-tunnel -" clear sentinel and the same platformCanaryOwnedRow refusal as |
| +// -channel/-environment, for the same structural reason. It writes through |
| +// its own SetTunnel call, independent of the -channel/-environment switch |
| +// below — with only ever one candidate, there is no combined transaction for |
| +// it to join. |
| // |
| // When BOTH -channel and -environment are given, both rails must resolve cleanly before EITHER |
| // axis writes: a single combined transaction (SetChannelsAndEnvironments) |
| @@ -338,12 +341,19 @@ func runRoute(args []string) { |
| if envClear { |
| resolvedEnv = "" |
| } |
| - // tunnelClear is the same "-" clear sentinel as -environment's. Unlike |
| - // -environment there is no reserved name and no bucket to validate |
| - // against here — a tunnel name only becomes real once the host-agent |
| - // dials in under it (later work in this feature), so there is nothing to |
| - // check existence of yet. |
| + // tunnelClear is the same "-" clear sentinel as -environment's, checked |
| + // before ValidTunnelName for the same reason envClear precedes |
| + // ValidEnvironmentName above: a bare hyphen fits [a-z0-9-]{1,32} and |
| + // would otherwise be treated as a real tunnel name. Unlike -environment |
| + // there is no reserved name and no bucket to validate EXISTENCE against |
| + // here — a tunnel name only becomes real once the host-agent dials in |
| + // under it (later work in this feature) — but the shape itself is |
| + // checked, same as -environment's, since this name is later matched |
| + // against a dialed-in identity and may build keys. |
| tunnelClear := *tunnel == "-" |
| + if *tunnel != "" && !tunnelClear && !registry.ValidTunnelName(*tunnel) { |
| + log.Fatalf("route: %q is not a valid tunnel name — want [a-z0-9-]{1,32}", *tunnel) |
| + } |
| resolvedTunnel := *tunnel |
| if tunnelClear { |
| resolvedTunnel = "" |
diff --git a/internal/registry/registry.go b/internal/registry/registry.go| index e04138c8a..9070d05ae 100644 |
| --- a/internal/registry/registry.go |
| +++ b/internal/registry/registry.go |
| @@ -12,6 +12,7 @@ package registry |
| import ( |
| "database/sql" |
| "fmt" |
| + "regexp" |
| "strings" |
| "time" |
| @@ -112,6 +113,26 @@ type Route struct { |
| // default so pre-existing rows keep today's behavior byte-identical. |
| const BackingUnit = "unit" |
| +// tunnelNameRe: lowercase letters, digits and hyphens, 1-32 characters — the |
| +// same shape as appconfig.ValidEnvironmentName, for the same reason: this |
| +// name travels through a CLI argument an operator types by hand, and later |
| +// work in the remote-host-agent feature (issue #53) matches it against a |
| +// dialed-in tunnel identity and may build keys from it — a permissive shape |
| +// (path separators, spaces, "..") would let a bad value reach the column |
| +// verbatim with nothing downstream able to retroactively fix it. |
| +var tunnelNameRe = regexp.MustCompile(`^[a-z0-9-]{1,32}$`) |
| + |
| +// ValidTunnelName reports whether name is usable as a route's Tunnel |
| +// binding. "" is refused as a NAME here — it is how callers ask for "no |
| +// tunnel" (Route.Tunnel's zero value), never a tunnel to bind to; the CLI's |
| +// "-tunnel -" clear sentinel resolves to "" before this is ever called on |
| +// it, the same way -environment's envClear does for ValidEnvironmentName. |
| +// Lives here rather than in a CLI or edge package because Tunnel is this |
| +// package's column — same precedent as BackingUnit above it. |
| +func ValidTunnelName(name string) bool { |
| + return tunnelNameRe.MatchString(name) |
| +} |
| + |
| func Open(path string) (*Registry, error) { |
| d, err := sql.Open("sqlite", path) |
| if err != nil { |
diff --git a/internal/registry/registry_test.go b/internal/registry/registry_test.go| index 66aaac68d..57fa2fd82 100644 |
| --- a/internal/registry/registry_test.go |
| +++ b/internal/registry/registry_test.go |
| @@ -831,3 +831,57 @@ func TestUpsertPreservesTunnel(t *testing.T) { |
| t.Fatalf("SetTunnel clear failed: %q", got.Tunnel) |
| } |
| } |
| + |
| +// Same contract as TestSetEnvironmentUnknownHostErrors: a human types a |
| +// hostname into `carlos route -tunnel` directly, and a silent no-op on a |
| +// typo would be worse than a refusal. |
| +func TestSetTunnelUnknownHostErrors(t *testing.T) { |
| + reg := openTestRegistry(t) |
| + if err := reg.SetTunnel("nope.example.com", "pi-box"); err == nil { |
| + t.Fatal("SetTunnel on an unknown host: got nil error, want one") |
| + } |
| +} |
| + |
| +// Mirrors appconfig.TestValidEnvironmentName: ValidTunnelName gates what |
| +// reaches the `tunnel` column before it does — a name later matched against |
| +// a dialed-in tunnel identity and used to build keys (remote-host-agent |
| +// feature, issue #53), so path separators, spaces, and "" must never reach |
| +// SetTunnel/Upsert unvalidated. Unlike ValidEnvironmentName there is no |
| +// reserved name (no "default" tunnel), so "" is invalid only because it |
| +// fails the character class, not a separate reservation. |
| +func TestValidTunnelName(t *testing.T) { |
| + valid := []string{"demo", "a", "pi-box", "connect-timeslots", "0-9"} |
| + invalid := []string{ |
| + "", // empty |
| + "Demo", // uppercase |
| + "has space", // space |
| + "has_us", // underscore not allowed |
| + "../../x", // path traversal |
| + "UP", |
| + string(make([]byte, 33)), // too long (33 NUL bytes, also fails the char class) |
| + } |
| + for _, n := range valid { |
| + if !ValidTunnelName(n) { |
| + t.Errorf("ValidTunnelName(%q) = false, want true", n) |
| + } |
| + } |
| + for _, n := range invalid { |
| + if ValidTunnelName(n) { |
| + t.Errorf("ValidTunnelName(%q) = true, want false", n) |
| + } |
| + } |
| +} |
| + |
| +func TestValidTunnelNameLengthBoundary(t *testing.T) { |
| + ok32 := "01234567890123456789012345678901" // 32 chars |
| + if len(ok32) != 32 { |
| + t.Fatalf("test fixture is %d chars, want 32", len(ok32)) |
| + } |
| + if !ValidTunnelName(ok32) { |
| + t.Errorf("ValidTunnelName(32 chars) = false, want true") |
| + } |
| + tooLong := ok32 + "x" |
| + if ValidTunnelName(tooLong) { |
| + t.Errorf("ValidTunnelName(33 chars) = true, want false") |
| + } |
| +} |