feat(security): allowed_private_cidrs — narrow the private-IP block (PR 1/2 for #337)

docs/security/egress-control.md

@@ -109,6 +109,32 @@ The `allowPrivateIPs` setting is resolved with this precedence:
 
 Cloud metadata (`169.254.169.254`) is **always** blocked regardless of the `allowPrivateIPs` setting.
 
+### Narrow Private-CIDR Allowlist (issue #337 preparation)
+
+`allow_private_ips: true` opens **all** RFC 1918 / link-local / CGNAT / ULA ranges at once — the right choice for a Kubernetes pod that talks to any service in the mesh, but too wide for an agent that only needs to reach one internal database.
+
+`allowed_private_cidrs` narrows that: when `allow_private_ips` is false (or unset), IPs falling inside any listed CIDR bypass the private-block, and everything else private stays blocked.
+
+```yaml
+egress:
+  mode: allowlist
+  allow_private_ips: false                # keep RFC 1918 blocked wholesale
+  allowed_private_cidrs:                  # ...but open these two slices
+    - 10.20.0.0/16                        # internal databases
+    - 172.16.42.0/24                      # message brokers
+```
+
+Invariants (checked by tests):
+
+- **Always-blocked ranges (cloud metadata `169.254.169.254`, loopback, `0.0.0.0/8`) stay blocked** even if an operator lists a CIDR that would otherwise contain them. No allowlist punches a hole in these.
+- **`allow_private_ips: true` supersedes** the CIDR list — the boolean means "all private allowed" and the list is redundant. A warning in that case is a good future addition, but the effective policy is safe (superset).
+- **Invalid CIDR strings fail at config-load time**, not at first dial. `Resolve()` validates each entry via `net.ParseCIDR`; bare IPs (missing `/mask`) are rejected because the config intent is range-level exemption. **Non-canonical entries with host bits set** (e.g. `10.20.0.5/16`) are also rejected — Go's `net.ParseCIDR` silently masks those to the network (widening from a single host to a /16), which is the security-wrong direction. Write `10.20.0.5/32` (single host) or `10.20.0.0/16` (range) — never the ambiguous mix.
+- **Both `forge build` and `forge run` validate the CIDR strings** — a typo trips the build, not just the first launch.
+- **`0.0.0.0/0` is not restricted.** Listing the whole IPv4 space here is equivalent to `allow_private_ips: true` in effect — cloud metadata / loopback / `0.0.0.0/8` still can't be reached because always-blocked wins. It's an explicit operator choice, not a hole, but if you find yourself writing it you probably want `allow_private_ips: true` for the same effect + clearer intent.
+- CIDRs are consulted by both the in-process `EgressEnforcer` (Go clients) and the subprocess `EgressProxy` (skill scripts) — one policy, two enforcement paths.
+
+Companion feature for #337: raw-TCP egress (databases, message brokers) requires this narrow-private mechanism to be reachable in the first place. Landing the CIDR allowlist first (this PR) unblocks the SOCKS5-based TCP path (follow-up PR) without opening RFC 1918 wholesale.
+
 ## Runtime Egress Enforcer
 
 The `EgressEnforcer` (`forge-core/security/egress_enforcer.go`) is an `http.RoundTripper` that wraps a `SafeTransport`. Every outbound HTTP request from in-process Go code (builtins like `http_request`, `web_search`, LLM API calls) passes through it.
@@ -307,10 +333,14 @@ egress:
     - slack
     - telegram
   allow_private_ips: false          # default: auto-detect from container env
+  allowed_private_cidrs:            # narrow private-IP allowlist (see above)
+    - 10.20.0.0/16
 ```
 
 The `allow_private_ips` field controls whether RFC 1918 addresses are allowed through the SafeDialer. When omitted, it defaults to `true` inside containers (detected via `KUBERNETES_SERVICE_HOST` or `/.dockerenv`) and `false` otherwise. Cloud metadata (`169.254.169.254`) is always blocked.
 
+`allowed_private_cidrs` (added in issue #337) is a narrower alternative: it lets you reach a specific slice of the private-IP space (e.g. only the internal-database subnet) without opening RFC 1918 wholesale. See "Narrow Private-CIDR Allowlist" above.
+
 ## Production vs Development
 
 | Setting | Production | Development |

forge-cli/build/egress_stage.go

@@ -51,7 +51,14 @@ func (s *EgressStage) Execute(ctx context.Context, bc *pipeline.BuildContext) er
 	// time out depending on which side notices first.
 	allowed = append(allowed, security.LLMProviderDomains(bc.Config)...)
 
-	resolved, err := security.Resolve(cfg.Profile, cfg.Mode, allowed, toolNames, cfg.Capabilities)
+	// Pass allowed_private_cidrs through so `forge build` validates the
+	// strings at build time (same fail-loud posture as `forge run`). The
+	// build artifacts themselves (`GenerateAllowlistJSON`,
+	// `GenerateK8sNetworkPolicy`) don't consume the CIDRs — runtime IP
+	// enforcement re-resolves from raw config — but a typo in
+	// `allowed_private_cidrs` should fail the build, not sail through and
+	// trip only on first `forge run`. (#348 review nit 1.)
+	resolved, err := security.Resolve(cfg.Profile, cfg.Mode, allowed, toolNames, cfg.Capabilities, cfg.AllowedPrivateCIDRs)
 	if err != nil {
 		return fmt.Errorf("resolving egress: %w", err)
 	}

forge-cli/runtime/runner.go

@@ -662,6 +662,7 @@ func (r *Runner) Run(ctx context.Context) error {
 		egressDomains,
 		egressToolNames,
 		r.cfg.Config.Egress.Capabilities,
+		r.cfg.Config.Egress.AllowedPrivateCIDRs,
 	)
 	if egressErr != nil {
 		r.logger.Warn("failed to resolve egress config, using default", map[string]any{"error": egressErr.Error()})
@@ -675,7 +676,16 @@ func (r *Runner) Run(ctx context.Context) error {
 			allowPrivateIPs = true
 		}
 
-		enforcer := security.NewEgressEnforcer(nil, egressCfg.Mode, egressCfg.AllDomains, allowPrivateIPs)
+		// Parse the CIDR allowlist here so the enforcer and proxy share one
+		// resolved slice. Resolve() already validated the strings, so this
+		// only fails on an internal programming error.
+		allowedPrivateCIDRs, cidrErr := security.ParsePrivateCIDRs(egressCfg.AllowedPrivateCIDRs)
+		if cidrErr != nil {
+			r.logger.Warn("failed to parse allowed_private_cidrs, ignoring", map[string]any{"error": cidrErr.Error()})
+			allowedPrivateCIDRs = nil
+		}
+
+		enforcer := security.NewEgressEnforcer(nil, egressCfg.Mode, egressCfg.AllDomains, allowPrivateIPs, allowedPrivateCIDRs)
 		enforcer.OnAttempt = func(ctx context.Context, domain string, allowed bool) {
 			event := coreruntime.AuditEgressAllowed
 			if !allowed {
@@ -723,7 +733,7 @@ func (r *Runner) Run(ctx context.Context) error {
 		browserActive := r.derivedBrowserConfig != nil
 		if (!security.InContainer() && egressCfg.Mode != security.ModeDevOpen) || browserActive {
 			matcher := security.NewDomainMatcher(egressCfg.Mode, egressCfg.AllDomains)
-			egressProxy = security.NewEgressProxy(matcher, allowPrivateIPs)
+			egressProxy = security.NewEgressProxy(matcher, allowPrivateIPs, allowedPrivateCIDRs)
 			egressProxy.OnAttempt = func(a security.EgressAttempt) {
 				event := coreruntime.AuditEgressAllowed
 				if !a.Allowed {

forge-cli/tools/browser/manager_test.go

@@ -44,7 +44,7 @@ func requireChromium(t *testing.T) string {
 // trivial expression. Returns nil only if the browser genuinely started.
 func probeBrowserLaunch(bin string) error {
 	matcher := security.NewDomainMatcher(security.ModeAllowlist, nil)
-	proxy := security.NewEgressProxy(matcher, false)
+	proxy := security.NewEgressProxy(matcher, false, nil)
 	proxyURL, err := proxy.Start(context.Background())
 	if err != nil {
 		return err
@@ -71,7 +71,7 @@ func probeBrowserLaunch(bin string) error {
 // OnAttempt calls. Returns the proxy URL and the attempts map (guarded by mu).
 func startProxy(t *testing.T, matcher *security.DomainMatcher) (string, *sync.Mutex, map[string]bool) {
 	t.Helper()
-	proxy := security.NewEgressProxy(matcher, false)
+	proxy := security.NewEgressProxy(matcher, false, nil)
 	mu := &sync.Mutex{}
 	attempts := map[string]bool{}
 	proxy.OnAttempt = func(a security.EgressAttempt) {

forge-cli/tools/browser/manual_demo_test.go

@@ -49,7 +49,7 @@ func TestManualBrowseDemo(t *testing.T) {
 	// Allowlist exactly the target host (plus its www/apex sibling), so this
 	// demonstrates real egress enforcement, not dev-open.
 	matcher := security.NewDomainMatcher(security.ModeAllowlist, []string{u.Hostname()})
-	proxy := security.NewEgressProxy(matcher, false)
+	proxy := security.NewEgressProxy(matcher, false, nil)
 	proxy.OnAttempt = func(a security.EgressAttempt) {
 		verdict := "ALLOW"
 		if !a.Allowed {

forge-core/forgecore.go

@@ -67,6 +67,7 @@ func Compile(req CompileRequest) (*CompileResult, error) {
 		req.Config.Egress.AllowedDomains,
 		toolNames,
 		nil, // capabilities resolved from profile
+		req.Config.Egress.AllowedPrivateCIDRs,
 	)
 	if err != nil {
 		return nil, err

forge-core/security/egress_enforcer.go

@@ -3,6 +3,7 @@ package security
 import (
 	"context"
 	"fmt"
+	"net"
 	"net/http"
 	"strings"
 )
@@ -13,24 +14,27 @@ type egressClientKey struct{}
 // EgressEnforcer is an http.RoundTripper that validates outbound requests
 // against a domain allowlist before forwarding them to the base transport.
 type EgressEnforcer struct {
-	base            http.RoundTripper
-	matcher         *DomainMatcher
-	AllowPrivateIPs bool
-	OnAttempt       func(ctx context.Context, domain string, allowed bool)
+	base                http.RoundTripper
+	matcher             *DomainMatcher
+	AllowPrivateIPs     bool
+	AllowedPrivateCIDRs []*net.IPNet
+	OnAttempt           func(ctx context.Context, domain string, allowed bool)
 }
 
 // NewEgressEnforcer creates a new EgressEnforcer wrapping the given base transport.
 // If base is nil, a SafeTransport is used instead of http.DefaultTransport.
 // Domains may include wildcard prefixes (e.g. "*.github.com") which match any subdomain.
-func NewEgressEnforcer(base http.RoundTripper, mode EgressMode, domains []string, allowPrivateIPs bool) *EgressEnforcer {
+// allowedPrivateCIDRs narrows the SafeDialer's private-IP block; see NewSafeDialer.
+func NewEgressEnforcer(base http.RoundTripper, mode EgressMode, domains []string, allowPrivateIPs bool, allowedPrivateCIDRs []*net.IPNet) *EgressEnforcer {
 	if base == nil {
-		base = NewSafeTransport(nil, allowPrivateIPs)
+		base = NewSafeTransport(nil, allowPrivateIPs, allowedPrivateCIDRs)
 	}
 
 	return &EgressEnforcer{
-		base:            base,
-		matcher:         NewDomainMatcher(mode, domains),
-		AllowPrivateIPs: allowPrivateIPs,
+		base:                base,
+		matcher:             NewDomainMatcher(mode, domains),
+		AllowPrivateIPs:     allowPrivateIPs,
+		AllowedPrivateCIDRs: allowedPrivateCIDRs,
 	}
 }
 

forge-core/security/egress_enforcer_test.go

@@ -92,7 +92,7 @@ func TestEgressEnforcerAllowlist(t *testing.T) {
 			}))
 			defer ts.Close()
 
-			enforcer := NewEgressEnforcer(http.DefaultTransport, ModeAllowlist, tt.domains, false)
+			enforcer := NewEgressEnforcer(http.DefaultTransport, ModeAllowlist, tt.domains, false, nil)
 
 			req, err := http.NewRequest("GET", tt.url, nil)
 			if err != nil {
@@ -119,7 +119,7 @@ func TestEgressEnforcerAllowlist(t *testing.T) {
 }
 
 func TestEgressEnforcerDenyAll(t *testing.T) {
-	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeDenyAll, nil, false)
+	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeDenyAll, nil, false, nil)
 
 	req, _ := http.NewRequest("GET", "https://api.openai.com/v1/chat", nil)
 	_, err := enforcer.RoundTrip(req)
@@ -134,7 +134,7 @@ func TestEgressEnforcerDenyAllAllowsLocalhost(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeDenyAll, nil, false)
+	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeDenyAll, nil, false, nil)
 
 	req, _ := http.NewRequest("GET", ts.URL+"/test", nil)
 	resp, err := enforcer.RoundTrip(req)
@@ -150,7 +150,7 @@ func TestEgressEnforcerDevOpen(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeDevOpen, nil, false)
+	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeDevOpen, nil, false, nil)
 
 	req, _ := http.NewRequest("GET", ts.URL+"/test", nil)
 	resp, err := enforcer.RoundTrip(req)
@@ -167,7 +167,7 @@ func TestEgressEnforcerOnAttemptCallback(t *testing.T) {
 		allowed bool
 	}
 
-	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeAllowlist, []string{"api.openai.com"}, false)
+	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeAllowlist, []string{"api.openai.com"}, false, nil)
 	enforcer.OnAttempt = func(_ context.Context, domain string, allowed bool) {
 		mu.Lock()
 		calls = append(calls, struct {
@@ -201,7 +201,7 @@ func TestEgressEnforcerOnAttemptCallback(t *testing.T) {
 
 func TestEgressEnforcerDevOpenCallback(t *testing.T) {
 	var called bool
-	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeDevOpen, nil, false)
+	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeDevOpen, nil, false, nil)
 	enforcer.OnAttempt = func(_ context.Context, domain string, allowed bool) {
 		called = true
 		if !allowed {
@@ -256,7 +256,7 @@ func TestEgressContextMissing(t *testing.T) {
 }
 
 func TestEgressEnforcerNilBase(t *testing.T) {
-	enforcer := NewEgressEnforcer(nil, ModeAllowlist, []string{"example.com"}, false)
+	enforcer := NewEgressEnforcer(nil, ModeAllowlist, []string{"example.com"}, false, nil)
 	if enforcer.base == nil {
 		t.Error("nil base should be replaced with SafeTransport")
 	}
@@ -285,7 +285,7 @@ func TestIsLocalhost(t *testing.T) {
 
 func TestEgressEnforcerSSRFBypass(t *testing.T) {
 	// Non-standard IP formats should be blocked by ValidateHostIP
-	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeAllowlist, []string{"example.com"}, false)
+	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeAllowlist, []string{"example.com"}, false, nil)
 
 	tests := []struct {
 		name string

forge-core/security/egress_integration_test.go

@@ -20,7 +20,7 @@ func TestEgressEnforcerIntegration(t *testing.T) {
 	defer ts.Close()
 
 	// Build an enforcer that allows only localhost (test server is localhost)
-	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeAllowlist, []string{"api.openai.com"}, false)
+	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeAllowlist, []string{"api.openai.com"}, false, nil)
 
 	var attemptLog []struct {
 		domain  string
@@ -78,7 +78,7 @@ func TestEgressEnforcerDenyAllIntegration(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeDenyAll, nil, false)
+	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeDenyAll, nil, false, nil)
 	client := &http.Client{Transport: enforcer}
 
 	// Localhost should still work even in deny-all
@@ -90,7 +90,7 @@ func TestEgressEnforcerDenyAllIntegration(t *testing.T) {
 }
 
 func TestEgressEnforcerWildcardIntegration(t *testing.T) {
-	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeAllowlist, []string{"*.example.com"}, false)
+	enforcer := NewEgressEnforcer(http.DefaultTransport, ModeAllowlist, []string{"*.example.com"}, false, nil)
 
 	var attempts []struct {
 		domain  string

forge-core/security/egress_proxy.go

@@ -47,13 +47,15 @@ type egressIdentity struct {
 }
 
 // NewEgressProxy creates a new EgressProxy that validates domains using the
-// given DomainMatcher. Call Start to bind and begin serving.
-func NewEgressProxy(matcher *DomainMatcher, allowPrivateIPs bool) *EgressProxy {
-	sd := NewSafeDialer(nil, allowPrivateIPs)
+// given DomainMatcher. Call Start to bind and begin serving. allowedPrivateCIDRs
+// narrows the private-IP block: when allowPrivateIPs is false, IPs inside any
+// of the listed CIDRs bypass the private block. Pass nil for pre-CIDR defaults.
+func NewEgressProxy(matcher *DomainMatcher, allowPrivateIPs bool, allowedPrivateCIDRs []*net.IPNet) *EgressProxy {
+	sd := NewSafeDialer(nil, allowPrivateIPs, allowedPrivateCIDRs)
 	return &EgressProxy{
 		matcher:       matcher,
 		safeDialer:    sd,
-		safeTransport: NewSafeTransport(nil, allowPrivateIPs),
+		safeTransport: NewSafeTransport(nil, allowPrivateIPs, allowedPrivateCIDRs),
 	}
 }
 

forge-core/security/egress_proxy_test.go

@@ -25,7 +25,7 @@ func TestEgressProxyAllowedHTTP(t *testing.T) {
 	upstreamURL, _ := url.Parse(upstream.URL)
 
 	matcher := NewDomainMatcher(ModeAllowlist, []string{upstreamURL.Hostname()})
-	proxy := NewEgressProxy(matcher, false)
+	proxy := NewEgressProxy(matcher, false, nil)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -58,7 +58,7 @@ func TestEgressProxyAllowedHTTP(t *testing.T) {
 
 func TestEgressProxyBlockedHTTP(t *testing.T) {
 	matcher := NewDomainMatcher(ModeAllowlist, []string{"allowed.com"})
-	proxy := NewEgressProxy(matcher, false)
+	proxy := NewEgressProxy(matcher, false, nil)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -91,7 +91,7 @@ func TestEgressProxyBlockedHTTP(t *testing.T) {
 func TestEgressProxyLocalhostAlwaysAllowed(t *testing.T) {
 	// Even with deny-all, localhost should pass
 	matcher := NewDomainMatcher(ModeDenyAll, nil)
-	proxy := NewEgressProxy(matcher, false)
+	proxy := NewEgressProxy(matcher, false, nil)
 
 	// Start a local test server
 	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -129,7 +129,7 @@ func TestEgressProxyLocalhostAlwaysAllowed(t *testing.T) {
 
 func TestEgressProxyCONNECTBlocked(t *testing.T) {
 	matcher := NewDomainMatcher(ModeAllowlist, []string{"allowed.com"})
-	proxy := NewEgressProxy(matcher, false)
+	proxy := NewEgressProxy(matcher, false, nil)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -168,7 +168,7 @@ func TestEgressProxyCONNECTAllowed(t *testing.T) {
 	host, port, _ := net.SplitHostPort(upstreamURL.Host)
 
 	matcher := NewDomainMatcher(ModeAllowlist, []string{host})
-	proxy := NewEgressProxy(matcher, false)
+	proxy := NewEgressProxy(matcher, false, nil)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -210,7 +210,7 @@ func TestEgressProxyDevOpenPassthrough(t *testing.T) {
 
 	// dev-open should allow everything
 	matcher := NewDomainMatcher(ModeDevOpen, nil)
-	proxy := NewEgressProxy(matcher, false)
+	proxy := NewEgressProxy(matcher, false, nil)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -249,7 +249,7 @@ func TestEgressProxyOnAttemptCallback(t *testing.T) {
 	upstreamURL, _ := url.Parse(upstream.URL)
 
 	matcher := NewDomainMatcher(ModeAllowlist, []string{upstreamURL.Hostname()})
-	proxy := NewEgressProxy(matcher, false)
+	proxy := NewEgressProxy(matcher, false, nil)
 
 	var mu sync.Mutex
 	var calls []EgressAttempt
@@ -299,7 +299,7 @@ func TestEgressProxyOnAttemptCallback(t *testing.T) {
 
 func TestEgressProxyStop(t *testing.T) {
 	matcher := NewDomainMatcher(ModeDevOpen, nil)
-	proxy := NewEgressProxy(matcher, false)
+	proxy := NewEgressProxy(matcher, false, nil)
 
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
@@ -321,7 +321,7 @@ func TestEgressProxyStop(t *testing.T) {
 }
 
 func TestEgressProxyURL(t *testing.T) {
-	proxy := NewEgressProxy(NewDomainMatcher(ModeDevOpen, nil), false)
+	proxy := NewEgressProxy(NewDomainMatcher(ModeDevOpen, nil), false, nil)
 	if proxy.ProxyURL() != "" {
 		t.Error("ProxyURL should be empty before Start")
 	}
@@ -395,7 +395,7 @@ func TestEgressProxyAttributesIdentity(t *testing.T) {
 	upstreamURL, _ := url.Parse(upstream.URL)
 
 	matcher := NewDomainMatcher(ModeAllowlist, []string{upstreamURL.Hostname()})
-	proxy := NewEgressProxy(matcher, false)
+	proxy := NewEgressProxy(matcher, false, nil)
 
 	var mu sync.Mutex
 	var got EgressAttempt
@@ -451,7 +451,7 @@ func TestEgressProxyAttributesIdentityCONNECT(t *testing.T) {
 	host, port, _ := net.SplitHostPort(upstreamURL.Host)
 
 	matcher := NewDomainMatcher(ModeAllowlist, []string{host})
-	proxy := NewEgressProxy(matcher, false)
+	proxy := NewEgressProxy(matcher, false, nil)
 
 	var mu sync.Mutex
 	var got EgressAttempt