fix(console): read the web allowlist per channel, not once at startup

cmd/agent/run.go

@@ -53,6 +53,8 @@ func runCmd(args []string) int {
 		fmt.Fprintf(os.Stderr, "run: no usable identity in %s (enroll first): %v\n", *dir, err)
 		return exitFailed
 	}
+	// Once here, to refuse a malformed file where an operator is watching;
+	// the handler reads it again per channel.
 	webAllow, err := console.LoadWebAllowlist(*webTargets)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "run: %v\n", err)
@@ -148,7 +150,7 @@ func runCmd(args []string) int {
 	srv := &agent.Server{
 		Tools:   reg,
 		Console: &console.Handler{NodeID: id.NodeID},
-		Web:     &console.WebHandler{Allow: webAllow},
+		Web:     &console.WebHandler{Allow: console.FileAllowlist{Path: *webTargets}},
 	}
 	backoff := tunnel.DefaultBackoff()
 	guard := tunnel.DefaultFlapGuard()

internal/console/web.go

@@ -53,10 +53,35 @@ func (a WebAllowlist) Resolve(name string) (string, error) {
 	return hostport, nil
 }
 
+// WebResolver is what a handler asks for an address. A fixed WebAllowlist is
+// one; so is a file read fresh on every channel.
+type WebResolver interface {
+	Resolve(name string) (string, error)
+}
+
+// FileAllowlist resolves against the file each time it is asked.
+//
+// The file changes underneath a running agent -- `advisor target_set` writes
+// it, and config_advisor's Commit seeds one -- and reading it once at startup
+// meant every such change needed a restart. A read per channel costs nothing:
+// a channel is opened by a person clicking a button.
+//
+// A parse error refuses rather than falling back to the last good copy: an
+// allowlist nobody can read is not one to keep enforcing from memory.
+type FileAllowlist struct{ Path string }
+
+func (f FileAllowlist) Resolve(name string) (string, error) {
+	a, err := LoadWebAllowlist(f.Path)
+	if err != nil {
+		return "", err
+	}
+	return a.Resolve(name)
+}
+
 // WebHandler serves web channels for one node.
 type WebHandler struct {
-	// Allow is the only source of addresses. Empty refuses everything.
-	Allow WebAllowlist
+	// Allow is the only source of addresses. Nil refuses everything.
+	Allow WebResolver
 
 	// Dial opens the upstream connection; nil uses a real TCP dialler.
 	Dial Dialer
@@ -67,6 +92,9 @@ type WebHandler struct {
 // Bytes only: the agent parses no HTTP, so there is no request it can be
 // tricked into rewriting.
 func (h *WebHandler) Serve(ctx context.Context, name string, rw io.ReadWriter) error {
+	if h.Allow == nil {
+		return fmt.Errorf("%w: %q", ErrNotAllowed, name)
+	}
 	hostport, err := h.Allow.Resolve(name)
 	if err != nil {
 		return err

internal/console/web_test.go

@@ -132,3 +132,50 @@ func TestAnUnlistedWebTargetIsNeverDialled(t *testing.T) {
 		t.Error("an unlisted target reached the dialler")
 	}
 }
+
+// Reading the file once at startup meant a target added later was refused
+// with "not in this node's web allowlist", naming one the file contained.
+func TestFileAllowlistSeesATargetAddedAfterStartup(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "web-targets.json")
+	write := func(body string) {
+		t.Helper()
+		if err := os.WriteFile(path, []byte(body), 0o644); err != nil {
+			t.Fatal(err)
+		}
+	}
+	write(`{"cube-cos":"10.0.0.1:443"}`)
+
+	a := console.FileAllowlist{Path: path}
+	if _, err := a.Resolve("cube-cos-idp"); !errors.Is(err, console.ErrNotAllowed) {
+		t.Fatalf("before it was added: err = %v, want ErrNotAllowed", err)
+	}
+
+	write(`{"cube-cos":"10.0.0.1:443","cube-cos-idp":"10.0.0.1:10443"}`)
+
+	got, err := a.Resolve("cube-cos-idp")
+	if err != nil {
+		t.Fatalf("after it was added: %v", err)
+	}
+	if got != "10.0.0.1:10443" {
+		t.Errorf("resolved to %q, want 10.0.0.1:10443", got)
+	}
+}
+
+// A file that stopped parsing refuses rather than falling back to whatever
+// was read last.
+func TestFileAllowlistRefusesWhenTheFileStopsParsing(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "web-targets.json")
+	if err := os.WriteFile(path, []byte(`{"cube-cos":"10.0.0.1:443"}`), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	a := console.FileAllowlist{Path: path}
+	if _, err := a.Resolve("cube-cos"); err != nil {
+		t.Fatalf("while it parsed: %v", err)
+	}
+	if err := os.WriteFile(path, []byte(`{ not json`), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := a.Resolve("cube-cos"); err == nil {
+		t.Fatal("a target resolved from an unreadable allowlist")
+	}
+}