From 1052f8bdb05c91f136a76e8078c372a04365d594 Mon Sep 17 00:00:00 2001
From: Hongming Wang <hongmingwang.rabbit@users.noreply.github.com>
Date: Tue, 5 May 2026 11:35:24 -0700
Subject: [PATCH] fix(memory-plugin): bind to 127.0.0.1 by default
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Self-review of PR #2906 flagged: defaultListenAddr was ":9100" — binds
on every container interface. Inside today's deployment that's moot
(no host port mapping, platform talks over loopback) but it's not
least-privilege. A future Dockerfile edit that publishes the port,
a misconfigured Fly machine, or a future cross-host plugin topology
would expose an unauth'd memory store.

Loopback is the right baseline. Operators with a multi-host topology
already override via MEMORY_PLUGIN_LISTEN_ADDR — that path is unchanged.

Tests:
  * TestLoadConfig_DefaultListenAddrIsLoopback pins the new default.
  * TestLoadConfig_ListenAddrEnvOverride pins the override path so
    operators relying on it don't break.
  * TestLoadConfig_MissingDatabaseURL covers the existing fail-fast.

No prior unit tests existed for loadConfig — boot_e2e_test.go always
sets MEMORY_PLUGIN_LISTEN_ADDR explicitly, so the default was never
exercised by tests. This PR adds that coverage.

Refs RFC #2728. Hardening follow-up to PR #2906.
---
 .../cmd/memory-plugin-postgres/config_test.go | 50 +++++++++++++++++++
 .../cmd/memory-plugin-postgres/main.go        |  8 ++-
 2 files changed, 57 insertions(+), 1 deletion(-)
 create mode 100644 workspace-server/cmd/memory-plugin-postgres/config_test.go

diff --git a/workspace-server/cmd/memory-plugin-postgres/config_test.go b/workspace-server/cmd/memory-plugin-postgres/config_test.go
new file mode 100644
index 00000000..252f0d1b
--- /dev/null
+++ b/workspace-server/cmd/memory-plugin-postgres/config_test.go
@@ -0,0 +1,50 @@
+package main
+
+import (
+	"strings"
+	"testing"
+)
+
+// TestLoadConfig_DefaultListenAddrIsLoopback pins the default-bind contract.
+//
+// Why this matters: with the prior `:9100` default, the plugin listened on
+// every interface. Inside the container it didn't matter (no host port
+// mapping today), but a future change that publishes 9100 OR a cross-host
+// sidecar deploy would have exposed an unauth'd memory store. Loopback by
+// default is the least-privilege baseline; operators with a multi-host
+// topology override via MEMORY_PLUGIN_LISTEN_ADDR.
+func TestLoadConfig_DefaultListenAddrIsLoopback(t *testing.T) {
+	t.Setenv("MEMORY_PLUGIN_DATABASE_URL", "postgres://stub")
+	t.Setenv("MEMORY_PLUGIN_LISTEN_ADDR", "")
+
+	cfg, err := loadConfig()
+	if err != nil {
+		t.Fatalf("loadConfig: %v", err)
+	}
+	if !strings.HasPrefix(cfg.ListenAddr, "127.0.0.1:") {
+		t.Errorf("default ListenAddr must bind loopback-only, got %q "+
+			"(security regression — would expose plugin on every interface)",
+			cfg.ListenAddr)
+	}
+}
+
+func TestLoadConfig_ListenAddrEnvOverride(t *testing.T) {
+	t.Setenv("MEMORY_PLUGIN_DATABASE_URL", "postgres://stub")
+	t.Setenv("MEMORY_PLUGIN_LISTEN_ADDR", ":9100")
+
+	cfg, err := loadConfig()
+	if err != nil {
+		t.Fatalf("loadConfig: %v", err)
+	}
+	if cfg.ListenAddr != ":9100" {
+		t.Errorf("env override ignored: want :9100, got %q", cfg.ListenAddr)
+	}
+}
+
+func TestLoadConfig_MissingDatabaseURL(t *testing.T) {
+	t.Setenv("MEMORY_PLUGIN_DATABASE_URL", "")
+
+	if _, err := loadConfig(); err == nil {
+		t.Fatal("loadConfig must error when MEMORY_PLUGIN_DATABASE_URL is empty")
+	}
+}
diff --git a/workspace-server/cmd/memory-plugin-postgres/main.go b/workspace-server/cmd/memory-plugin-postgres/main.go
index 84e01351..148c1dd4 100644
--- a/workspace-server/cmd/memory-plugin-postgres/main.go
+++ b/workspace-server/cmd/memory-plugin-postgres/main.go
@@ -31,7 +31,13 @@ const (
 	envListenAddr  = "MEMORY_PLUGIN_LISTEN_ADDR"
 	envSkipMigrate = "MEMORY_PLUGIN_SKIP_MIGRATE"
 
-	defaultListenAddr = ":9100"
+	// Loopback-only by default (defense in depth). The platform talks to
+	// the plugin over `http://localhost:9100` from the same container, so
+	// binding to all interfaces would only widen the reachable surface
+	// without enabling any in-design caller. Operators running the plugin
+	// on a separate host override via MEMORY_PLUGIN_LISTEN_ADDR=:9100 (or
+	// some other interface).
+	defaultListenAddr = "127.0.0.1:9100"
 )
 
 func main() {