From 2c49873e169d44df4cd20dd75614579f35f1c96a Mon Sep 17 00:00:00 2001 From: yen <5915590+antigenius0910@users.noreply.github.com> Date: Tue, 14 Jul 2026 00:44:52 -0500 Subject: [PATCH 1/8] feat(chart,k8s): add optional NetworkPolicy for pod-level isolation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an opt-in NetworkPolicy for openab agent and gateway pods. Default networkPolicy.enabled=false — no resource is rendered, existing releases are unaffected. Mirrors the opt-in pattern of #901 / #911 / #914. Two supported egress topologies via networkPolicy.egress.mode: - direct (default): DNS + HTTPS to 0.0.0.0/0 (except 169.254.169.254/32, the cloud metadata service) — hobbyist / dev-cluster path. - proxy: DNS + egress restricted to a designated proxy Service via networkPolicy.egress.proxy.{namespace, podSelector, ports} — enterprise path per @pahud's Discord feedback citing OpenSHELL prior art. Chart ships no proxy; users BYO (OpenSHELL, Squid, Envoy, mitmproxy, etc.), keeping upstream maintenance surface unchanged. Rendered resources when enabled: - -: deny-all ingress, egress per mode. - --gateway: adds port-8080 ingress when agents..gateway.enabled=true. Selector labels reuse openab.selectorLabels so drift with deployment.yaml and gateway.yaml is structurally impossible. Also adds k8s/networkpolicy.yaml mirroring the chart's direct-mode default (deny-all ingress, DNS + HTTPS egress) so the raw manifests set cited by the driving AppSec finding is covered too. Tests: 8 helm-unittest cases covering both modes, gateway split, extraRules, and locked-down egress. Full chart suite (48 tests) still passes; helm lint clean. Closes #1394 --- charts/openab/README.md | 8 + charts/openab/templates/networkpolicy.yaml | 116 ++++++++++ charts/openab/tests/networkpolicy_test.yaml | 241 ++++++++++++++++++++ charts/openab/values.yaml | 58 +++++ k8s/networkpolicy.yaml | 33 +++ 5 files changed, 456 insertions(+) create mode 100644 charts/openab/templates/networkpolicy.yaml create mode 100644 charts/openab/tests/networkpolicy_test.yaml create mode 100644 k8s/networkpolicy.yaml diff --git a/charts/openab/README.md b/charts/openab/README.md index dfac33a38..2baeae52f 100644 --- a/charts/openab/README.md +++ b/charts/openab/README.md @@ -14,6 +14,14 @@ This page highlights commonly used values and deployment patterns. For the compl | `fullnameOverride` | Override the full generated release name for chart resources. Useful when deploying multiple instances with predictable names. | `""` | | `serviceAccountName` | Chart-global ServiceAccount name attached to every agent pod that doesn't define its own. Empty = cluster `default` SA. Per-agent `agents..serviceAccountName` fully overrides this. Chart references an existing SA only — does not create one. Required for workload identity and pod-level RBAC. | `""` | | `imagePullSecrets` | Chart-global image pull secrets attached to every agent pod that doesn't define its own. Per-agent `agents..imagePullSecrets` fully overrides this. | `[]` | +| `networkPolicy.enabled` | Master switch for the chart-managed NetworkPolicy. Off by default — no `NetworkPolicy` resource is rendered until this flips to `true`, keeping existing releases unaffected. When on: one policy per agent (deny-all ingress + configured egress), plus one policy per gateway pod (port 8080 ingress + same egress) when `agents..gateway.enabled` is `true`. | `false` | +| `networkPolicy.ingress.extraRules` | Raw `NetworkPolicyIngressRule` entries appended verbatim to every rendered policy. Use for cluster-specific allow-lists (Prometheus scrape, mesh sidecars, etc.). | `[]` | +| `networkPolicy.egress.mode` | Egress topology. `direct` allows egress to the open internet (DNS + HTTPS by default). `proxy` restricts egress to a designated proxy Service — recommended for enterprise / multi-tenant / compliance-driven deployments (see [RFC #1394](https://github.com/openabdev/openab/issues/1394) and OpenSHELL prior art). Chart does not ship the proxy — BYO. | `"direct"` | +| `networkPolicy.egress.allowDns` | Allow DNS egress to pods labelled `k8s-app=kube-dns` (kubeadm / CoreDNS default). Applies in both `direct` and `proxy` modes. | `true` | +| `networkPolicy.egress.allowHttps` | (direct mode only) Allow HTTPS egress to `0.0.0.0/0` except the cloud metadata service `169.254.169.254/32`. Ignored in proxy mode. | `true` | +| `networkPolicy.egress.allowHttp` | (direct mode only) Allow HTTP egress with the same metadata exclusion. Off by default; enable if hooks / STT fetch over HTTP. Ignored in proxy mode. | `false` | +| `networkPolicy.egress.proxy` | (proxy mode only) Selector for the egress proxy Service — `{namespace, podSelector, ports}`. When `mode=proxy`, agent and gateway egress is restricted to this selector. Deploy the proxy separately (OpenSHELL, Squid, Envoy, mitmproxy, etc.). | `{namespace: "", podSelector: {}, ports: []}` | +| `networkPolicy.egress.extraRules` | Raw `NetworkPolicyEgressRule` entries appended verbatim. Use to open extra ports or add cluster-specific carve-outs. | `[]` | ### Agent values diff --git a/charts/openab/templates/networkpolicy.yaml b/charts/openab/templates/networkpolicy.yaml new file mode 100644 index 000000000..dcd09bd5d --- /dev/null +++ b/charts/openab/templates/networkpolicy.yaml @@ -0,0 +1,116 @@ +{{- if .Values.networkPolicy.enabled }} +{{- $np := .Values.networkPolicy }} +{{- range $name, $cfg := .Values.agents }} +{{- if ne (include "openab.agentEnabled" $cfg) "false" }} +{{- $d := dict "ctx" $ "agent" $name "cfg" $cfg }} +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: {{ include "openab.agentFullname" $d }} + labels: + {{- include "openab.labels" $d | nindent 4 }} +spec: + podSelector: + matchLabels: + {{- include "openab.selectorLabels" $d | nindent 6 }} + policyTypes: + - Ingress + - Egress + ingress: + {{- with $np.ingress.extraRules }} + {{- toYaml . | nindent 4 }} + {{- else }} + [] + {{- end }} + egress: +{{ include "openab.networkPolicyEgress" (dict "np" $np) | indent 4 }} +{{- if and (($cfg.gateway).enabled) (ne (($cfg.gateway).deploy | default true | toString) "false") }} +{{- $gwCfg := omit $cfg "nameOverride" }} +{{- $gwD := dict "ctx" $ "agent" (printf "%s-gateway" $name) "cfg" $gwCfg }} +--- +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: {{ include "openab.agentFullname" $gwD }} + labels: + {{- include "openab.labels" $gwD | nindent 4 }} +spec: + podSelector: + matchLabels: + {{- include "openab.selectorLabels" $gwD | nindent 6 }} + policyTypes: + - Ingress + - Egress + ingress: + - ports: + - protocol: TCP + port: 8080 + {{- with $np.ingress.extraRules }} + {{- toYaml . | nindent 4 }} + {{- end }} + egress: +{{ include "openab.networkPolicyEgress" (dict "np" $np) | indent 4 }} +{{- end }} +{{- end }} +{{- end }} +{{- end }} + +{{/* Egress rule set shared by agent and gateway NetworkPolicies. + Call with: dict "np" .Values.networkPolicy */}} +{{- define "openab.networkPolicyEgress" -}} +{{- $np := .np -}} +{{- if $np.egress.allowDns -}} +- to: + - namespaceSelector: {} + podSelector: + matchLabels: + k8s-app: kube-dns + ports: + - protocol: UDP + port: 53 + - protocol: TCP + port: 53 +{{- end }} +{{- if eq $np.egress.mode "proxy" }} +- to: +{{- if $np.egress.proxy.namespace }} + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: {{ $np.egress.proxy.namespace }} + podSelector: + {{- toYaml $np.egress.proxy.podSelector | nindent 8 }} +{{- else }} + - podSelector: + {{- toYaml $np.egress.proxy.podSelector | nindent 8 }} +{{- end }} +{{- with $np.egress.proxy.ports }} + ports: + {{- toYaml . | nindent 4 }} +{{- end }} +{{- else }} +{{- if $np.egress.allowHttps }} +- to: + - ipBlock: + cidr: 0.0.0.0/0 + except: + - 169.254.169.254/32 + ports: + - protocol: TCP + port: 443 +{{- end }} +{{- if $np.egress.allowHttp }} +- to: + - ipBlock: + cidr: 0.0.0.0/0 + except: + - 169.254.169.254/32 + ports: + - protocol: TCP + port: 80 +{{- end }} +{{- end }} +{{- with $np.egress.extraRules }} +{{ toYaml . }} +{{- end }} +{{- end }} diff --git a/charts/openab/tests/networkpolicy_test.yaml b/charts/openab/tests/networkpolicy_test.yaml new file mode 100644 index 000000000..326e0cd28 --- /dev/null +++ b/charts/openab/tests/networkpolicy_test.yaml @@ -0,0 +1,241 @@ +suite: NetworkPolicy support (opt-in, direct + proxy egress modes) +templates: + - templates/networkpolicy.yaml + +tests: + - it: renders no NetworkPolicy resource when networkPolicy.enabled is false (default) + asserts: + - hasDocuments: + count: 0 + + - it: renders a single agent NetworkPolicy in direct mode (gateway off) with deny-all ingress and DNS + HTTPS egress + set: + networkPolicy.enabled: true + asserts: + - hasDocuments: + count: 1 + - equal: + path: kind + value: NetworkPolicy + - equal: + path: metadata.name + value: RELEASE-NAME-openab-kiro + - equal: + path: spec.podSelector.matchLabels["app.kubernetes.io/component"] + value: kiro + - equal: + path: spec.policyTypes + value: [Ingress, Egress] + - equal: + path: spec.ingress + value: [] + - contains: + path: spec.egress + content: + to: + - namespaceSelector: {} + podSelector: + matchLabels: + k8s-app: kube-dns + ports: + - protocol: UDP + port: 53 + - protocol: TCP + port: 53 + - contains: + path: spec.egress + content: + to: + - ipBlock: + cidr: 0.0.0.0/0 + except: + - 169.254.169.254/32 + ports: + - protocol: TCP + port: 443 + + - it: renders both agent and gateway NetworkPolicies in direct mode when gateway is enabled + set: + networkPolicy.enabled: true + agents.kiro.gateway.enabled: true + asserts: + - hasDocuments: + count: 2 + - equal: + path: metadata.name + value: RELEASE-NAME-openab-kiro + documentIndex: 0 + - equal: + path: metadata.name + value: RELEASE-NAME-openab-kiro-gateway + documentIndex: 1 + - equal: + path: spec.podSelector.matchLabels["app.kubernetes.io/component"] + value: kiro-gateway + documentIndex: 1 + - contains: + path: spec.ingress + content: + ports: + - protocol: TCP + port: 8080 + documentIndex: 1 + + - it: renders HTTP egress rule when egress.allowHttp is true (direct mode) + set: + networkPolicy.enabled: true + networkPolicy.egress.allowHttp: true + asserts: + - contains: + path: spec.egress + content: + to: + - ipBlock: + cidr: 0.0.0.0/0 + except: + - 169.254.169.254/32 + ports: + - protocol: TCP + port: 80 + + - it: restricts agent egress to the designated proxy Service in proxy mode + set: + networkPolicy.enabled: true + networkPolicy.egress.mode: proxy + networkPolicy.egress.proxy.namespace: egress-proxy + networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell + networkPolicy.egress.proxy.ports: + - protocol: TCP + port: 3128 + asserts: + - contains: + path: spec.egress + content: + to: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: egress-proxy + podSelector: + matchLabels: + app: openshell + ports: + - protocol: TCP + port: 3128 + - notContains: + path: spec.egress + content: + to: + - ipBlock: + cidr: 0.0.0.0/0 + except: + - 169.254.169.254/32 + ports: + - protocol: TCP + port: 443 + any: true + + - it: applies proxy-mode egress to both agent and gateway policies when gateway is enabled + set: + networkPolicy.enabled: true + networkPolicy.egress.mode: proxy + networkPolicy.egress.proxy.namespace: egress-proxy + networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell + networkPolicy.egress.proxy.ports: + - protocol: TCP + port: 3128 + agents.kiro.gateway.enabled: true + asserts: + - hasDocuments: + count: 2 + - contains: + path: spec.egress + content: + to: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: egress-proxy + podSelector: + matchLabels: + app: openshell + ports: + - protocol: TCP + port: 3128 + documentIndex: 0 + - contains: + path: spec.egress + content: + to: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: egress-proxy + podSelector: + matchLabels: + app: openshell + ports: + - protocol: TCP + port: 3128 + documentIndex: 1 + + - it: appends ingress.extraRules verbatim to the agent policy + set: + networkPolicy.enabled: true + networkPolicy.ingress.extraRules: + - from: + - namespaceSelector: + matchLabels: + name: monitoring + ports: + - protocol: TCP + port: 9090 + asserts: + - contains: + path: spec.ingress + content: + from: + - namespaceSelector: + matchLabels: + name: monitoring + ports: + - protocol: TCP + port: 9090 + + - it: locks egress down to only extraRules when allowDns and allowHttps are false (direct mode) + set: + networkPolicy.enabled: true + networkPolicy.egress.allowDns: false + networkPolicy.egress.allowHttps: false + networkPolicy.egress.extraRules: + - to: + - ipBlock: + cidr: 10.0.0.0/8 + ports: + - protocol: TCP + port: 443 + asserts: + - notContains: + path: spec.egress + content: + to: + - namespaceSelector: {} + podSelector: + matchLabels: + k8s-app: kube-dns + any: true + - notContains: + path: spec.egress + content: + to: + - ipBlock: + cidr: 0.0.0.0/0 + except: + - 169.254.169.254/32 + any: true + - contains: + path: spec.egress + content: + to: + - ipBlock: + cidr: 10.0.0.0/8 + ports: + - protocol: TCP + port: 443 diff --git a/charts/openab/values.yaml b/charts/openab/values.yaml index 34c99c9ab..7d1d9447b 100644 --- a/charts/openab/values.yaml +++ b/charts/openab/values.yaml @@ -43,6 +43,64 @@ containerSecurityContext: drop: - ALL +# NetworkPolicy for pod-level network isolation. Opt-in and backwards-compatible: +# no NetworkPolicy resource is rendered unless networkPolicy.enabled=true. +# See RFC: https://github.com/openabdev/openab/issues/1394 +networkPolicy: + enabled: false + + # Ingress rules. Default = deny-all ingress to agent pods (they only + # initiate outbound Socket Mode / websocket connections). Gateway pod + # gets a separate policy allowing port 8080 when gateway.enabled=true. + ingress: + # extraRules: raw NetworkPolicyIngressRule entries appended verbatim. + # Use for cluster-specific allow-lists (e.g. Prometheus scrape, mesh sidecars). + extraRules: [] + + egress: + # Egress topology. "direct" (default) allows egress to the open internet + # (typically DNS + HTTPS). "proxy" restricts egress to a designated proxy + # Service — recommended for enterprise / multi-tenant / compliance-driven + # deployments (see OpenSHELL prior art). + mode: direct + + # Allow DNS to kube-dns (UDP+TCP 53). Almost always required in both modes. + # Targets pods labelled k8s-app=kube-dns in any namespace (kubeadm / CoreDNS default). + # For non-standard DNS setups, disable this and add an appropriate egress.extraRules entry. + allowDns: true + + # ── mode: direct ──────────────────────────────────────────────────────── + # Allow HTTPS (TCP 443) to 0.0.0.0/0 EXCEPT the cloud metadata service + # (169.254.169.254/32) — blocks a common IAM-credential-theft path from + # a compromised agent pod. Ignored when mode=proxy. + allowHttps: true + # Allow HTTP (TCP 80) with the same metadata exclusion. Off by default; + # enable if hooks / STT fetch over HTTP. Ignored when mode=proxy. + allowHttp: false + + # ── mode: proxy ───────────────────────────────────────────────────────── + # When mode=proxy, egress is restricted to this Service selector. + # The chart does NOT ship the proxy — deploy it separately (OpenSHELL, + # Squid, Envoy, mitmproxy, or anything speaking HTTP CONNECT). + # egress.allowDns above still applies. allowHttps/allowHttp are IGNORED. + # Example: + # proxy: + # namespace: "egress-proxy" + # podSelector: + # matchLabels: + # app: openshell + # ports: + # - protocol: TCP + # port: 3128 + proxy: + namespace: "" + podSelector: {} + ports: [] + + # extraRules: raw NetworkPolicyEgressRule entries appended verbatim. + # Use to open extra ports or add cluster-specific carve-outs. + extraRules: [] + agents: kiro: enabled: true # set to false to skip creating resources for this agent diff --git a/k8s/networkpolicy.yaml b/k8s/networkpolicy.yaml new file mode 100644 index 000000000..b7e008932 --- /dev/null +++ b/k8s/networkpolicy.yaml @@ -0,0 +1,33 @@ +apiVersion: networking.k8s.io/v1 +kind: NetworkPolicy +metadata: + name: openab + labels: + app: openab +spec: + podSelector: + matchLabels: + app: openab + policyTypes: + - Ingress + - Egress + ingress: [] + egress: + - to: + - namespaceSelector: {} + podSelector: + matchLabels: + k8s-app: kube-dns + ports: + - protocol: UDP + port: 53 + - protocol: TCP + port: 53 + - to: + - ipBlock: + cidr: 0.0.0.0/0 + except: + - 169.254.169.254/32 + ports: + - protocol: TCP + port: 443 From d1a7ff0fe7bce0216fcec2bbc68ea36c886ea847 Mon Sep 17 00:00:00 2001 From: yen <5915590+antigenius0910@users.noreply.github.com> Date: Tue, 14 Jul 2026 17:36:59 -0500 Subject: [PATCH 2/8] =?UTF-8?q?fix(chart,k8s):=20address=20PR=20#1400=20re?= =?UTF-8?q?view=20=E2=80=94=20fail-closed=20proxy,=20gateway=20path,=20DNS?= =?UTF-8?q?=20scoping,=20IPv6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all 9 findings raised on the initial commit. Details map 1:1 to the chaodu-agent review on PR #1400. Critical (fail-closed): - F1 Validate networkPolicy.egress.mode + required proxy fields at render time. mode must be exactly "direct" or "proxy" (typos like "proxxy" no longer silently fall back to direct). mode=proxy now requires non-empty proxy.podSelector AND non-empty proxy.ports, otherwise an empty selector authorizes every pod on every port (previous fail-open behaviour). - F2 Add agent→gateway egress rule on TCP 8080 when a chart-managed gateway is deployed (agents..gateway.enabled=true AND gateway.deploy is not "false"). Without this, enabling the policy breaks the ws://--gateway:8080/ws data path. Important: - F3 Match templates/gateway.yaml condition for gateway.deploy — the previous `default true | toString` treated an explicit false as empty and rendered an orphan gateway NetworkPolicy with no Deployment. - F4 Scope default DNS destination to the trusted kube-system namespace (kubernetes.io/metadata.name=kube-system) instead of any namespace. Prevents a co-tenant labelling a pod k8s-app=kube-dns from becoming an approved port-53 destination. New egress.dnsNamespace value (default "kube-system", empty = legacy any-namespace behaviour). Same tightening applied to k8s/networkpolicy.yaml. - F5 Document the proxy-mode contract explicitly in values.yaml and charts/openab/README.md: NetworkPolicy enforces the destination allowlist only. Operators must configure HTTPS_PROXY/HTTP_PROXY via agents..env AND add them to inherit_env in configToml so ACP children inherit the settings. Chart does not auto-inject. - F6 Add opt-in `kubectl apply -f k8s/networkpolicy.yaml` step and manifest-table row to README.md and README.zh-TW.md so users following the raw-install path aren't left unisolated. - F7 Quote the proxy namespace label value so all-numeric namespace names (e.g. "1234") render as strings, not YAML integers. - F8 Add optional IPv6 support (allowIpv6, default true). Adds ::/0 alongside 0.0.0.0/0 in HTTPS/HTTP rules, excluding fd00:ec2::254/128 (AWS IMDS IPv6 pattern). Non-AWS IPv6 metadata addresses vary; docs point users at extraRules for cluster-specific exclusions. Same addition in k8s/networkpolicy.yaml. Toggle off for IPv4-only clusters. - F9 Update allowHttp documentation in values.yaml and chart README to explicitly call out HTTP configUrl as an affected use case; recommend HTTPS configUrl by default. Tests: 15 helm-unittest cases (up from 8). New coverage: - 3 negative tests for F1 validation (invalid mode, empty podSelector, empty ports) - agent→gateway egress path assertion (F2) - gateway.enabled=true+deploy=false → no gateway NP, no agent egress (F3) - DNS namespace scoping default + empty-string escape hatch (F4) - numeric proxy namespace renders as string (F7) - IPv6 rendered by default, omitted when allowIpv6=false (F8) Full chart suite: 55/55 pass, no regression. helm lint clean. Refs #1394 Refs #1400 review by chaodu-agent --- README.md | 6 + README.zh-TW.md | 6 + charts/openab/README.md | 14 +- charts/openab/templates/networkpolicy.yaml | 63 +++++++- charts/openab/tests/networkpolicy_test.yaml | 161 +++++++++++++++++++- charts/openab/values.yaml | 81 +++++++--- k8s/networkpolicy.yaml | 15 +- 7 files changed, 299 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index a97a527aa..d8a0f81e8 100644 --- a/README.md +++ b/README.md @@ -272,6 +272,11 @@ kubectl create secret generic openab-secret \ kubectl apply -f k8s/configmap.yaml kubectl apply -f k8s/pvc.yaml kubectl apply -f k8s/deployment.yaml + +# Optional: opt-in pod-level network isolation (deny-all ingress, +# DNS + HTTPS egress with cloud-metadata exclusion). See the file +# header for tunable CIDRs / IPv6 exclusions per cloud. +kubectl apply -f k8s/networkpolicy.yaml ``` | Manifest | Purpose | @@ -280,6 +285,7 @@ kubectl apply -f k8s/deployment.yaml | `k8s/configmap.yaml` | `config.toml` mounted at `/etc/openab/` | | `k8s/secret.yaml` | `DISCORD_BOT_TOKEN` injected as env var | | `k8s/pvc.yaml` | Persistent storage for auth + settings | +| `k8s/networkpolicy.yaml` | **Optional** pod-level network isolation — deny-all ingress + DNS/HTTPS egress with cloud-metadata (`169.254.169.254`, AWS `fd00:ec2::254`) excluded. Requires a CNI that enforces NetworkPolicy (Calico, Cilium, etc.). | ## AWS ECS Deployment diff --git a/README.zh-TW.md b/README.zh-TW.md index ed6c2330a..2d29eeab5 100644 --- a/README.zh-TW.md +++ b/README.zh-TW.md @@ -272,6 +272,11 @@ kubectl create secret generic openab-secret \ kubectl apply -f k8s/configmap.yaml kubectl apply -f k8s/pvc.yaml kubectl apply -f k8s/deployment.yaml + +# 選用：pod 層網路隔離（deny-all ingress、DNS + HTTPS egress，並排除 +# cloud metadata service）。若需要調整 CIDR 或不同雲的 IPv6 metadata +# 位址，請直接修改該 manifest 檔案開頭的註解。 +kubectl apply -f k8s/networkpolicy.yaml ``` | Manifest | 用途 | @@ -280,6 +285,7 @@ kubectl apply -f k8s/deployment.yaml | `k8s/configmap.yaml` | `config.toml` 掛載至 `/etc/openab/` | | `k8s/secret.yaml` | 透過 env 注入 `DISCORD_BOT_TOKEN` | | `k8s/pvc.yaml` | 持久保存驗證資訊與設定 | +| `k8s/networkpolicy.yaml` | **選用**：pod 層網路隔離 — deny-all ingress + DNS/HTTPS egress，並排除 cloud metadata（`169.254.169.254`、AWS `fd00:ec2::254`）。需要有 CNI 執行 NetworkPolicy（Calico、Cilium 等）。 | ## AWS ECS 部署 diff --git a/charts/openab/README.md b/charts/openab/README.md index 2baeae52f..088cfe07d 100644 --- a/charts/openab/README.md +++ b/charts/openab/README.md @@ -14,13 +14,15 @@ This page highlights commonly used values and deployment patterns. For the compl | `fullnameOverride` | Override the full generated release name for chart resources. Useful when deploying multiple instances with predictable names. | `""` | | `serviceAccountName` | Chart-global ServiceAccount name attached to every agent pod that doesn't define its own. Empty = cluster `default` SA. Per-agent `agents..serviceAccountName` fully overrides this. Chart references an existing SA only — does not create one. Required for workload identity and pod-level RBAC. | `""` | | `imagePullSecrets` | Chart-global image pull secrets attached to every agent pod that doesn't define its own. Per-agent `agents..imagePullSecrets` fully overrides this. | `[]` | -| `networkPolicy.enabled` | Master switch for the chart-managed NetworkPolicy. Off by default — no `NetworkPolicy` resource is rendered until this flips to `true`, keeping existing releases unaffected. When on: one policy per agent (deny-all ingress + configured egress), plus one policy per gateway pod (port 8080 ingress + same egress) when `agents..gateway.enabled` is `true`. | `false` | +| `networkPolicy.enabled` | Master switch for the chart-managed NetworkPolicy. Off by default — no `NetworkPolicy` resource is rendered until this flips to `true`, keeping existing releases unaffected. When on: one policy per agent (deny-all ingress + configured egress + TCP 8080 to the chart-managed gateway pod when `gateway.enabled=true` and `gateway.deploy` is not `false`), plus one policy per deployed gateway (port 8080 ingress + same egress). | `false` | | `networkPolicy.ingress.extraRules` | Raw `NetworkPolicyIngressRule` entries appended verbatim to every rendered policy. Use for cluster-specific allow-lists (Prometheus scrape, mesh sidecars, etc.). | `[]` | -| `networkPolicy.egress.mode` | Egress topology. `direct` allows egress to the open internet (DNS + HTTPS by default). `proxy` restricts egress to a designated proxy Service — recommended for enterprise / multi-tenant / compliance-driven deployments (see [RFC #1394](https://github.com/openabdev/openab/issues/1394) and OpenSHELL prior art). Chart does not ship the proxy — BYO. | `"direct"` | -| `networkPolicy.egress.allowDns` | Allow DNS egress to pods labelled `k8s-app=kube-dns` (kubeadm / CoreDNS default). Applies in both `direct` and `proxy` modes. | `true` | -| `networkPolicy.egress.allowHttps` | (direct mode only) Allow HTTPS egress to `0.0.0.0/0` except the cloud metadata service `169.254.169.254/32`. Ignored in proxy mode. | `true` | -| `networkPolicy.egress.allowHttp` | (direct mode only) Allow HTTP egress with the same metadata exclusion. Off by default; enable if hooks / STT fetch over HTTP. Ignored in proxy mode. | `false` | -| `networkPolicy.egress.proxy` | (proxy mode only) Selector for the egress proxy Service — `{namespace, podSelector, ports}`. When `mode=proxy`, agent and gateway egress is restricted to this selector. Deploy the proxy separately (OpenSHELL, Squid, Envoy, mitmproxy, etc.). | `{namespace: "", podSelector: {}, ports: []}` | +| `networkPolicy.egress.mode` | Egress topology. Must be exactly `"direct"` or `"proxy"` — invalid values fail the render (fail-closed). `direct` allows egress to the open internet (DNS + HTTPS by default). `proxy` restricts egress to a designated proxy Service — recommended for enterprise / multi-tenant / compliance-driven deployments (see [RFC #1394](https://github.com/openabdev/openab/issues/1394) and OpenSHELL prior art). **Proxy mode enforces the destination allowlist only; you must also configure your pods to send traffic through the proxy (e.g. set `HTTPS_PROXY` / `HTTP_PROXY` via `agents..env` and add them to `inherit_env` in `configToml` so ACP children inherit).** Chart does not ship the proxy — BYO (OpenSHELL / Squid / Envoy / mitmproxy). | `"direct"` | +| `networkPolicy.egress.allowDns` | Allow DNS egress on UDP+TCP 53 to pods labelled `k8s-app=kube-dns`. Applies in both `direct` and `proxy` modes. | `true` | +| `networkPolicy.egress.dnsNamespace` | Trusted namespace hosting the DNS pods. Default `"kube-system"` matches kubeadm / CoreDNS / kube-dns defaults and prevents a co-tenant labelling their own pod `k8s-app=kube-dns` from becoming an approved port-53 destination. Empty string reverts to the legacy any-namespace selector — use only if your DNS runs outside kube-system. | `"kube-system"` | +| `networkPolicy.egress.allowHttps` | (direct mode only) Allow HTTPS (TCP 443) to `0.0.0.0/0` except cloud metadata (`169.254.169.254/32`, and `fd00:ec2::254/128` when `allowIpv6=true`). Ignored in proxy mode. | `true` | +| `networkPolicy.egress.allowHttp` | (direct mode only) Allow HTTP (TCP 80) with the same metadata exclusion. Off by default. ⚠️ Enable if any of these fetch over HTTP: `agents..configUrl`, lifecycle hooks that download over HTTP (see `docs/hooks.md`), or STT base URLs on HTTP. Prefer HTTPS wherever possible. Ignored in proxy mode. | `false` | +| `networkPolicy.egress.allowIpv6` | (direct mode only) Add `::/0` to the HTTPS/HTTP rules alongside `0.0.0.0/0`, with AWS IMDS IPv6 (`fd00:ec2::254/128`) excluded. Non-AWS clouds use different IPv6 metadata addresses — override via `extraRules` for a tighter exclusion. Turn off on IPv4-only clusters if you prefer explicit denial. Ignored in proxy mode. | `true` | +| `networkPolicy.egress.proxy` | (proxy mode only) Selector for the egress proxy Service — `{namespace, podSelector, ports}`. Render fails if `podSelector` is empty or `ports` is empty (would otherwise authorize every pod on every port — fail-open). Deploy the proxy separately. | `{namespace: "", podSelector: {}, ports: []}` | | `networkPolicy.egress.extraRules` | Raw `NetworkPolicyEgressRule` entries appended verbatim. Use to open extra ports or add cluster-specific carve-outs. | `[]` | ### Agent values diff --git a/charts/openab/templates/networkpolicy.yaml b/charts/openab/templates/networkpolicy.yaml index dcd09bd5d..62ed10aa4 100644 --- a/charts/openab/templates/networkpolicy.yaml +++ b/charts/openab/templates/networkpolicy.yaml @@ -1,8 +1,24 @@ {{- if .Values.networkPolicy.enabled }} {{- $np := .Values.networkPolicy }} +{{/* ---------- F1: mode and proxy-required-fields validation ---------- */}} +{{- $mode := $np.egress.mode | default "direct" }} +{{- if not (or (eq $mode "direct") (eq $mode "proxy")) }} +{{- fail (printf "networkPolicy.egress.mode must be exactly \"direct\" or \"proxy\" (got %q)" $mode) }} +{{- end }} +{{- if eq $mode "proxy" }} +{{- if not (or $np.egress.proxy.podSelector.matchLabels $np.egress.proxy.podSelector.matchExpressions) }} +{{- fail "networkPolicy.egress.mode=proxy requires networkPolicy.egress.proxy.podSelector.matchLabels or matchExpressions to be non-empty (an empty selector authorizes every pod in the namespace)" }} +{{- end }} +{{- if not $np.egress.proxy.ports }} +{{- fail "networkPolicy.egress.mode=proxy requires networkPolicy.egress.proxy.ports to be non-empty (an empty ports list authorizes every port)" }} +{{- end }} +{{- end }} {{- range $name, $cfg := .Values.agents }} {{- if ne (include "openab.agentEnabled" $cfg) "false" }} {{- $d := dict "ctx" $ "agent" $name "cfg" $cfg }} +{{/* F2: chart-managed gateway is deployed iff gateway.enabled AND gateway.deploy != false (matches templates/gateway.yaml) */}} +{{- $gwDeployed := and (($cfg.gateway).enabled) (ne (($cfg.gateway).deploy | toString) "false") }} +{{- $gwD := dict "ctx" $ "agent" (printf "%s-gateway" $name) "cfg" (omit $cfg "nameOverride") }} --- apiVersion: networking.k8s.io/v1 kind: NetworkPolicy @@ -25,9 +41,17 @@ spec: {{- end }} egress: {{ include "openab.networkPolicyEgress" (dict "np" $np) | indent 4 }} -{{- if and (($cfg.gateway).enabled) (ne (($cfg.gateway).deploy | default true | toString) "false") }} -{{- $gwCfg := omit $cfg "nameOverride" }} -{{- $gwD := dict "ctx" $ "agent" (printf "%s-gateway" $name) "cfg" $gwCfg }} +{{- if $gwDeployed }} + - to: + - podSelector: + matchLabels: + {{- include "openab.selectorLabels" $gwD | nindent 14 }} + ports: + - protocol: TCP + port: 8080 +{{- end }} +{{- if $gwDeployed }} +{{/* F3: use the same deploy check as templates/gateway.yaml — avoid orphan NPs */}} --- apiVersion: networking.k8s.io/v1 kind: NetworkPolicy @@ -61,33 +85,43 @@ spec: {{- define "openab.networkPolicyEgress" -}} {{- $np := .np -}} {{- if $np.egress.allowDns -}} +{{/* F4: default DNS destination scoped to a trusted namespace (usually kube-system). + Empty dnsNamespace preserves the legacy any-namespace behaviour for exotic DNS setups. */}} - to: +{{- if $np.egress.dnsNamespace }} + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: {{ $np.egress.dnsNamespace | quote }} + podSelector: + matchLabels: + k8s-app: kube-dns +{{- else }} - namespaceSelector: {} podSelector: matchLabels: k8s-app: kube-dns +{{- end }} ports: - protocol: UDP port: 53 - protocol: TCP port: 53 {{- end }} -{{- if eq $np.egress.mode "proxy" }} +{{- if eq ($np.egress.mode | default "direct") "proxy" }} - to: {{- if $np.egress.proxy.namespace }} +{{/* F7: quote the namespace value — bare all-numeric namespace names YAML-parse as int */}} - namespaceSelector: matchLabels: - kubernetes.io/metadata.name: {{ $np.egress.proxy.namespace }} + kubernetes.io/metadata.name: {{ $np.egress.proxy.namespace | quote }} podSelector: {{- toYaml $np.egress.proxy.podSelector | nindent 8 }} {{- else }} - podSelector: {{- toYaml $np.egress.proxy.podSelector | nindent 8 }} {{- end }} -{{- with $np.egress.proxy.ports }} ports: - {{- toYaml . | nindent 4 }} -{{- end }} + {{- toYaml $np.egress.proxy.ports | nindent 4 }} {{- else }} {{- if $np.egress.allowHttps }} - to: @@ -95,6 +129,13 @@ spec: cidr: 0.0.0.0/0 except: - 169.254.169.254/32 +{{- if $np.egress.allowIpv6 }} +{{/* F8: IPv6 IMDS excludes the AWS pattern; other clouds may need extraRules */}} + - ipBlock: + cidr: ::/0 + except: + - fd00:ec2::254/128 +{{- end }} ports: - protocol: TCP port: 443 @@ -105,6 +146,12 @@ spec: cidr: 0.0.0.0/0 except: - 169.254.169.254/32 +{{- if $np.egress.allowIpv6 }} + - ipBlock: + cidr: ::/0 + except: + - fd00:ec2::254/128 +{{- end }} ports: - protocol: TCP port: 80 diff --git a/charts/openab/tests/networkpolicy_test.yaml b/charts/openab/tests/networkpolicy_test.yaml index 326e0cd28..c6175bd36 100644 --- a/charts/openab/tests/networkpolicy_test.yaml +++ b/charts/openab/tests/networkpolicy_test.yaml @@ -8,7 +8,7 @@ tests: - hasDocuments: count: 0 - - it: renders a single agent NetworkPolicy in direct mode (gateway off) with deny-all ingress and DNS + HTTPS egress + - it: renders a single agent NetworkPolicy in direct mode (gateway off) with deny-all ingress, DNS scoped to kube-system, and IPv4+IPv6 HTTPS egress set: networkPolicy.enabled: true asserts: @@ -33,7 +33,9 @@ tests: path: spec.egress content: to: - - namespaceSelector: {} + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: kube-system podSelector: matchLabels: k8s-app: kube-dns @@ -50,11 +52,15 @@ tests: cidr: 0.0.0.0/0 except: - 169.254.169.254/32 + - ipBlock: + cidr: ::/0 + except: + - fd00:ec2::254/128 ports: - protocol: TCP port: 443 - - it: renders both agent and gateway NetworkPolicies in direct mode when gateway is enabled + - it: renders both agent and gateway NetworkPolicies in direct mode when gateway is deployed, and the agent policy authorizes TCP 8080 to the gateway pod (F2) set: networkPolicy.enabled: true agents.kiro.gateway.enabled: true @@ -80,8 +86,46 @@ tests: - protocol: TCP port: 8080 documentIndex: 1 + - contains: + path: spec.egress + content: + to: + - podSelector: + matchLabels: + app.kubernetes.io/name: openab + app.kubernetes.io/instance: RELEASE-NAME + app.kubernetes.io/component: kiro-gateway + ports: + - protocol: TCP + port: 8080 + documentIndex: 0 - - it: renders HTTP egress rule when egress.allowHttp is true (direct mode) + - it: does NOT render a gateway NetworkPolicy or an agent→gateway egress rule when gateway.enabled=true but gateway.deploy=false (F3 regression — matches templates/gateway.yaml condition) + set: + networkPolicy.enabled: true + agents.kiro.gateway.enabled: true + agents.kiro.gateway.deploy: false + asserts: + - hasDocuments: + count: 1 + - equal: + path: metadata.name + value: RELEASE-NAME-openab-kiro + - notContains: + path: spec.egress + content: + to: + - podSelector: + matchLabels: + app.kubernetes.io/name: openab + app.kubernetes.io/instance: RELEASE-NAME + app.kubernetes.io/component: kiro-gateway + ports: + - protocol: TCP + port: 8080 + any: true + + - it: renders HTTP egress rule (IPv4+IPv6) when egress.allowHttp is true (direct mode) set: networkPolicy.enabled: true networkPolicy.egress.allowHttp: true @@ -94,11 +138,60 @@ tests: cidr: 0.0.0.0/0 except: - 169.254.169.254/32 + - ipBlock: + cidr: ::/0 + except: + - fd00:ec2::254/128 ports: - protocol: TCP port: 80 - - it: restricts agent egress to the designated proxy Service in proxy mode + - it: omits IPv6 ipBlock when allowIpv6=false (direct mode, IPv4-only cluster) (F8) + set: + networkPolicy.enabled: true + networkPolicy.egress.allowIpv6: false + asserts: + - contains: + path: spec.egress + content: + to: + - ipBlock: + cidr: 0.0.0.0/0 + except: + - 169.254.169.254/32 + ports: + - protocol: TCP + port: 443 + - notContains: + path: spec.egress + content: + to: + - ipBlock: + cidr: ::/0 + except: + - fd00:ec2::254/128 + any: true + + - it: falls back to any-namespace DNS when egress.dnsNamespace is empty string (F4 legacy escape hatch) + set: + networkPolicy.enabled: true + networkPolicy.egress.dnsNamespace: "" + asserts: + - contains: + path: spec.egress + content: + to: + - namespaceSelector: {} + podSelector: + matchLabels: + k8s-app: kube-dns + ports: + - protocol: UDP + port: 53 + - protocol: TCP + port: 53 + + - it: restricts agent egress to the designated proxy Service in proxy mode (F5 — enforcement only) set: networkPolicy.enabled: true networkPolicy.egress.mode: proxy @@ -129,12 +222,30 @@ tests: cidr: 0.0.0.0/0 except: - 169.254.169.254/32 + - ipBlock: + cidr: ::/0 + except: + - fd00:ec2::254/128 ports: - protocol: TCP port: 443 any: true - - it: applies proxy-mode egress to both agent and gateway policies when gateway is enabled + - it: quotes an all-numeric proxy namespace so it renders as a string, not a YAML integer (F7) + set: + networkPolicy.enabled: true + networkPolicy.egress.mode: proxy + networkPolicy.egress.proxy.namespace: "1234" + networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell + networkPolicy.egress.proxy.ports: + - protocol: TCP + port: 3128 + asserts: + - equal: + path: spec.egress[1].to[0].namespaceSelector.matchLabels["kubernetes.io/metadata.name"] + value: "1234" + + - it: applies proxy-mode egress to both agent and gateway policies when gateway is deployed set: networkPolicy.enabled: true networkPolicy.egress.mode: proxy @@ -216,7 +327,9 @@ tests: path: spec.egress content: to: - - namespaceSelector: {} + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: kube-system podSelector: matchLabels: k8s-app: kube-dns @@ -229,6 +342,10 @@ tests: cidr: 0.0.0.0/0 except: - 169.254.169.254/32 + - ipBlock: + cidr: ::/0 + except: + - fd00:ec2::254/128 any: true - contains: path: spec.egress @@ -239,3 +356,33 @@ tests: ports: - protocol: TCP port: 443 + + # ── F1 negative tests: fail-closed validation ───────────────────────────── + + - it: fails rendering when egress.mode is not exactly "direct" or "proxy" (F1) + set: + networkPolicy.enabled: true + networkPolicy.egress.mode: proxxy + asserts: + - failedTemplate: + errorMessage: 'networkPolicy.egress.mode must be exactly "direct" or "proxy" (got "proxxy")' + + - it: fails rendering when mode=proxy but proxy.podSelector is empty (F1) + set: + networkPolicy.enabled: true + networkPolicy.egress.mode: proxy + networkPolicy.egress.proxy.ports: + - protocol: TCP + port: 3128 + asserts: + - failedTemplate: + errorMessage: 'networkPolicy.egress.mode=proxy requires networkPolicy.egress.proxy.podSelector.matchLabels or matchExpressions to be non-empty (an empty selector authorizes every pod in the namespace)' + + - it: fails rendering when mode=proxy but proxy.ports is empty (F1) + set: + networkPolicy.enabled: true + networkPolicy.egress.mode: proxy + networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell + asserts: + - failedTemplate: + errorMessage: 'networkPolicy.egress.mode=proxy requires networkPolicy.egress.proxy.ports to be non-empty (an empty ports list authorizes every port)' diff --git a/charts/openab/values.yaml b/charts/openab/values.yaml index 7d1d9447b..1d2a13e77 100644 --- a/charts/openab/values.yaml +++ b/charts/openab/values.yaml @@ -58,43 +58,74 @@ networkPolicy: extraRules: [] egress: - # Egress topology. "direct" (default) allows egress to the open internet - # (typically DNS + HTTPS). "proxy" restricts egress to a designated proxy - # Service — recommended for enterprise / multi-tenant / compliance-driven - # deployments (see OpenSHELL prior art). + # Egress topology. Must be exactly "direct" or "proxy" — invalid values + # fail the render (fail-closed). + # - direct: allow DNS + HTTPS (and optionally HTTP) to the open internet + # with cloud metadata (169.254.169.254 / fd00:ec2::254) excluded. + # Hobbyist / dev-cluster / small-team path. + # - proxy: allow DNS + egress restricted to a designated proxy Service. + # Enterprise path per OpenSHELL prior art. See "proxy contract" note below. mode: direct - # Allow DNS to kube-dns (UDP+TCP 53). Almost always required in both modes. - # Targets pods labelled k8s-app=kube-dns in any namespace (kubeadm / CoreDNS default). - # For non-standard DNS setups, disable this and add an appropriate egress.extraRules entry. + # Allow DNS egress on UDP/TCP 53 to pods labelled k8s-app=kube-dns. + # Applies in both direct and proxy modes. allowDns: true + # Trusted namespace hosting the DNS pods. Default "kube-system" matches + # kubeadm/CoreDNS/kube-dns defaults and prevents a co-tenant from labelling + # their own pod k8s-app=kube-dns to become an approved port-53 destination. + # Empty string reverts to "any namespace" (legacy behaviour) — use only if + # your DNS runs outside kube-system, or set to your DNS namespace directly. + dnsNamespace: "kube-system" # ── mode: direct ──────────────────────────────────────────────────────── - # Allow HTTPS (TCP 443) to 0.0.0.0/0 EXCEPT the cloud metadata service - # (169.254.169.254/32) — blocks a common IAM-credential-theft path from - # a compromised agent pod. Ignored when mode=proxy. + # Allow HTTPS (TCP 443). Excludes cloud metadata service (169.254.169.254/32 + # for IPv4, fd00:ec2::254/128 for AWS IPv6) — blocks a common IAM-credential- + # theft path from a compromised agent pod. Ignored when mode=proxy. allowHttps: true - # Allow HTTP (TCP 80) with the same metadata exclusion. Off by default; - # enable if hooks / STT fetch over HTTP. Ignored when mode=proxy. + # Allow HTTP (TCP 80) with the same metadata exclusion. Off by default. + # ⚠️ Enable if any of these fetch over HTTP: `agents..configUrl`, + # lifecycle hooks that download over HTTP (see docs/hooks.md), or STT + # base URLs on HTTP. Prefer HTTPS wherever possible. Ignored when mode=proxy. allowHttp: false + # Allow IPv6 egress in direct mode by adding ::/0 alongside 0.0.0.0/0 in + # the HTTPS/HTTP rules, with AWS IMDS IPv6 (fd00:ec2::254/128) excluded. + # ⚠️ Non-AWS clouds use different IPv6 metadata addresses; override via + # extraRules if you need a tighter exclusion. Turn off on IPv4-only + # clusters if you prefer explicit denial to allowing unused v6 ranges. + allowIpv6: true # ── mode: proxy ───────────────────────────────────────────────────────── - # When mode=proxy, egress is restricted to this Service selector. - # The chart does NOT ship the proxy — deploy it separately (OpenSHELL, - # Squid, Envoy, mitmproxy, or anything speaking HTTP CONNECT). - # egress.allowDns above still applies. allowHttps/allowHttp are IGNORED. - # Example: - # proxy: - # namespace: "egress-proxy" - # podSelector: - # matchLabels: - # app: openshell - # ports: - # - protocol: TCP - # port: 3128 + # ⚠️ Proxy-mode CONTRACT + # NetworkPolicy only ENFORCES the destination allowlist. It does NOT + # redirect traffic through the proxy. You must configure your pods to + # actually send traffic through the proxy — one common way is env vars: + # + # agents..env: + # HTTPS_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" + # HTTP_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" + # NO_PROXY: ".svc,.cluster.local,10.0.0.0/8" + # + # And add these variables to inherit_env in your configToml so ACP children + # (e.g. claude-agent-acp, kiro-cli, opencode) inherit the proxy settings: + # + # [agent] + # inherit_env = ["HTTPS_PROXY","HTTP_PROXY","NO_PROXY"] + # + # Without both pieces, agent traffic will hit the NetworkPolicy egress deny + # and fail to connect. Chart does NOT ship the proxy — deploy it separately + # (OpenSHELL, Squid, Envoy, mitmproxy, or anything speaking HTTP CONNECT). + # + # egress.allowDns above still applies. allowHttps/allowHttp/allowIpv6 are + # IGNORED when mode=proxy. + # + # Rendering fails if proxy.podSelector is empty or proxy.ports is empty + # (would otherwise authorize every pod on every port — fail-open). proxy: + # Namespace hosting the proxy pods. Empty = same namespace as the release. namespace: "" + # Kubernetes label selector for the proxy pods. podSelector: {} + # Ports on the proxy pods that agents should reach. ports: [] # extraRules: raw NetworkPolicyEgressRule entries appended verbatim. diff --git a/k8s/networkpolicy.yaml b/k8s/networkpolicy.yaml index b7e008932..76f886c1f 100644 --- a/k8s/networkpolicy.yaml +++ b/k8s/networkpolicy.yaml @@ -13,8 +13,12 @@ spec: - Egress ingress: [] egress: + # DNS scoped to kube-system to prevent a co-tenant from labelling + # their own pod k8s-app=kube-dns to become an approved port-53 destination. - to: - - namespaceSelector: {} + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: "kube-system" podSelector: matchLabels: k8s-app: kube-dns @@ -23,11 +27,20 @@ spec: port: 53 - protocol: TCP port: 53 + # HTTPS to the open internet, IPv4 and IPv6. Cloud metadata service + # excluded (169.254.169.254/32 for IPv4, AWS pattern fd00:ec2::254/128 + # for IPv6) to block a common IAM-credential-theft path from a compromised + # pod. Non-AWS IPv6 metadata addresses may need a tighter exclusion — + # edit this manifest for your environment. - to: - ipBlock: cidr: 0.0.0.0/0 except: - 169.254.169.254/32 + - ipBlock: + cidr: ::/0 + except: + - fd00:ec2::254/128 ports: - protocol: TCP port: 443 From f323386f0beece0800eaee9317f54a648c8d96cc Mon Sep 17 00:00:00 2001 From: yen <5915590+antigenius0910@users.noreply.github.com> Date: Tue, 14 Jul 2026 18:17:11 -0500 Subject: [PATCH 3/8] =?UTF-8?q?fix(chart):=20address=20PR=20#1400=20round-?= =?UTF-8?q?2=20review=20=E2=80=94=20gateway=20proxy=20env,=20configurable?= =?UTF-8?q?=20metadata=20exclusions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses both round-2 findings from chaodu-agent on the follow-up commit (d1a7ff0). Kept as a third commit rather than amend to preserve review history. Critical (round-2 F1) — Complete the proxy contract for the gateway: The previous commit locked the chart-managed gateway to proxy-only egress when networkPolicy.egress.mode=proxy, but the gateway Deployment had no mechanism to receive proxy env vars — its env list was a fixed platform-specific block only. Effect: proxy-mode gateway pods could reach DNS + the proxy pod but nothing else, so all outbound platform API calls (Telegram replies, Teams OAuth token exchange, Feishu/WeCom callbacks) silently failed. Fix: add first-class `agents..gateway.env` (map) and `agents..gateway.envFrom` (list) fields to values.yaml and render them into templates/gateway.yaml, right after the fixed platform env block. This lets operators inject HTTPS_PROXY / HTTP_PROXY / NO_PROXY into the gateway pod the same way they do for the agent. Updated the "Proxy-mode CONTRACT" doc block in values.yaml (and the matching chart README row) to spell out that operators must configure BOTH the agent AND the gateway to route through the proxy, with a concrete example config for each side. Important (round-2 F2) — Configurable IPv4/IPv6 metadata exclusions: The previous docs suggested users could tighten IPv6 metadata protection via `egress.extraRules`. That is incorrect — Kubernetes NetworkPolicy allow rules combine additively, so an extraRule can only broaden what is allowed, never subtract from an existing `::/0` allow. Non-AWS clouds with different IPv6 IMDS addresses would have been silently unprotected. Fix: new `networkPolicy.egress.metadataExclusions.{ipv4,ipv6}` value. Defaults preserve the AWS pattern (169.254.169.254/32 for IPv4, fd00:ec2::254/128 for IPv6). Users on other clouds edit the list directly (e.g. add IBM Cloud's 161.26.0.0/16 to ipv4, or override ipv6 for GCP/Azure). The template consumes both lists via `toYaml . | nindent`, so the exclusion set can grow as needed. Updated values.yaml comment block to enumerate common cloud IMDS addresses. Updated chart README rows for allowHttps/allowHttp/allowIpv6/ extraRules to point at metadataExclusions and clarify the additive semantics constraint. Tests: 18 helm-unittest cases in NetworkPolicy suite (up from 15). New: - gateway.env renders HTTPS_PROXY/HTTP_PROXY/NO_PROXY into the gateway Deployment container (round-2 F1) - gateway.envFrom renders on the gateway Deployment (round-2 F1) - custom metadataExclusions.ipv4/ipv6 render as expected `except` entries in the direct-mode HTTPS rule (round-2 F2) Full chart suite: 58/58 pass. helm lint clean. Refs #1394 Refs #1400 round-2 review by chaodu-agent --- charts/openab/README.md | 12 ++- charts/openab/templates/gateway.yaml | 8 ++ charts/openab/templates/networkpolicy.yaml | 14 ++-- charts/openab/tests/networkpolicy_test.yaml | 88 +++++++++++++++++++++ charts/openab/values.yaml | 81 ++++++++++++++----- 5 files changed, 173 insertions(+), 30 deletions(-) diff --git a/charts/openab/README.md b/charts/openab/README.md index 088cfe07d..3845a5ed0 100644 --- a/charts/openab/README.md +++ b/charts/openab/README.md @@ -16,14 +16,16 @@ This page highlights commonly used values and deployment patterns. For the compl | `imagePullSecrets` | Chart-global image pull secrets attached to every agent pod that doesn't define its own. Per-agent `agents..imagePullSecrets` fully overrides this. | `[]` | | `networkPolicy.enabled` | Master switch for the chart-managed NetworkPolicy. Off by default — no `NetworkPolicy` resource is rendered until this flips to `true`, keeping existing releases unaffected. When on: one policy per agent (deny-all ingress + configured egress + TCP 8080 to the chart-managed gateway pod when `gateway.enabled=true` and `gateway.deploy` is not `false`), plus one policy per deployed gateway (port 8080 ingress + same egress). | `false` | | `networkPolicy.ingress.extraRules` | Raw `NetworkPolicyIngressRule` entries appended verbatim to every rendered policy. Use for cluster-specific allow-lists (Prometheus scrape, mesh sidecars, etc.). | `[]` | -| `networkPolicy.egress.mode` | Egress topology. Must be exactly `"direct"` or `"proxy"` — invalid values fail the render (fail-closed). `direct` allows egress to the open internet (DNS + HTTPS by default). `proxy` restricts egress to a designated proxy Service — recommended for enterprise / multi-tenant / compliance-driven deployments (see [RFC #1394](https://github.com/openabdev/openab/issues/1394) and OpenSHELL prior art). **Proxy mode enforces the destination allowlist only; you must also configure your pods to send traffic through the proxy (e.g. set `HTTPS_PROXY` / `HTTP_PROXY` via `agents..env` and add them to `inherit_env` in `configToml` so ACP children inherit).** Chart does not ship the proxy — BYO (OpenSHELL / Squid / Envoy / mitmproxy). | `"direct"` | +| `networkPolicy.egress.mode` | Egress topology. Must be exactly `"direct"` or `"proxy"` — invalid values fail the render (fail-closed). `direct` allows egress to the open internet (DNS + HTTPS by default). `proxy` restricts egress to a designated proxy Service — recommended for enterprise / multi-tenant / compliance-driven deployments (see [RFC #1394](https://github.com/openabdev/openab/issues/1394) and OpenSHELL prior art). **Proxy mode enforces the destination allowlist only; you must also configure your workloads to send traffic through the proxy: set `HTTPS_PROXY` / `HTTP_PROXY` on the agent via `agents..env` + `inherit_env` in `configToml`, AND on the gateway via `agents..gateway.env` (the gateway makes its own outbound HTTPS calls to platform APIs).** Chart does not ship the proxy — BYO (OpenSHELL / Squid / Envoy / mitmproxy). | `"direct"` | | `networkPolicy.egress.allowDns` | Allow DNS egress on UDP+TCP 53 to pods labelled `k8s-app=kube-dns`. Applies in both `direct` and `proxy` modes. | `true` | | `networkPolicy.egress.dnsNamespace` | Trusted namespace hosting the DNS pods. Default `"kube-system"` matches kubeadm / CoreDNS / kube-dns defaults and prevents a co-tenant labelling their own pod `k8s-app=kube-dns` from becoming an approved port-53 destination. Empty string reverts to the legacy any-namespace selector — use only if your DNS runs outside kube-system. | `"kube-system"` | -| `networkPolicy.egress.allowHttps` | (direct mode only) Allow HTTPS (TCP 443) to `0.0.0.0/0` except cloud metadata (`169.254.169.254/32`, and `fd00:ec2::254/128` when `allowIpv6=true`). Ignored in proxy mode. | `true` | +| `networkPolicy.egress.allowHttps` | (direct mode only) Allow HTTPS (TCP 443) to `0.0.0.0/0` (and `::/0` when `allowIpv6=true`), except CIDRs listed in `metadataExclusions`. Ignored in proxy mode. | `true` | | `networkPolicy.egress.allowHttp` | (direct mode only) Allow HTTP (TCP 80) with the same metadata exclusion. Off by default. ⚠️ Enable if any of these fetch over HTTP: `agents..configUrl`, lifecycle hooks that download over HTTP (see `docs/hooks.md`), or STT base URLs on HTTP. Prefer HTTPS wherever possible. Ignored in proxy mode. | `false` | -| `networkPolicy.egress.allowIpv6` | (direct mode only) Add `::/0` to the HTTPS/HTTP rules alongside `0.0.0.0/0`, with AWS IMDS IPv6 (`fd00:ec2::254/128`) excluded. Non-AWS clouds use different IPv6 metadata addresses — override via `extraRules` for a tighter exclusion. Turn off on IPv4-only clusters if you prefer explicit denial. Ignored in proxy mode. | `true` | +| `networkPolicy.egress.allowIpv6` | (direct mode only) Add `::/0` to the HTTPS/HTTP rules alongside `0.0.0.0/0`, excluding CIDRs from `metadataExclusions.ipv6`. Turn off on IPv4-only clusters. Ignored in proxy mode. | `true` | +| `networkPolicy.egress.metadataExclusions.ipv4` | CIDRs excluded from the direct-mode IPv4 HTTPS/HTTP allow rule. Defaults to `169.254.169.254/32` (AWS/GCP/Azure IMDS). Add IBM Cloud's `161.26.0.0/16` here if applicable. **NetworkPolicy allow rules combine additively — you cannot subtract via `extraRules`; edit this list directly.** | `["169.254.169.254/32"]` | +| `networkPolicy.egress.metadataExclusions.ipv6` | CIDRs excluded from the direct-mode IPv6 HTTPS/HTTP allow rule. Defaults to `fd00:ec2::254/128` (AWS IPv6 IMDS). Non-AWS IPv6 metadata addresses differ — override this list per cloud. **Same additive-only semantics as `ipv4` above.** | `["fd00:ec2::254/128"]` | | `networkPolicy.egress.proxy` | (proxy mode only) Selector for the egress proxy Service — `{namespace, podSelector, ports}`. Render fails if `podSelector` is empty or `ports` is empty (would otherwise authorize every pod on every port — fail-open). Deploy the proxy separately. | `{namespace: "", podSelector: {}, ports: []}` | -| `networkPolicy.egress.extraRules` | Raw `NetworkPolicyEgressRule` entries appended verbatim. Use to open extra ports or add cluster-specific carve-outs. | `[]` | +| `networkPolicy.egress.extraRules` | Raw `NetworkPolicyEgressRule` entries appended verbatim. Use to open extra ports or add cluster-specific carve-outs. **Cannot subtract from existing allows — for tighter metadata exclusions use `metadataExclusions` above.** | `[]` | ### Agent values @@ -58,6 +60,8 @@ Each agent lives under `agents.`. | `stt.baseUrl` | STT API base URL. | `"https://api.groq.com/openai/v1"` | | `gateway.enabled` | Enable the gateway config block for webhook-based platforms. | `false` | | `gateway.deploy` | Deploy the gateway Deployment and Service. | `true` | +| `gateway.env` | Additional environment variables for the gateway container (map, `{name: value}`). Common use: `HTTPS_PROXY` / `HTTP_PROXY` / `NO_PROXY` when running under `networkPolicy.egress.mode=proxy` so the gateway can reach platform APIs (Telegram, Teams, etc.) through the same proxy as the agent. | `{}` | +| `gateway.envFrom` | Load env vars for the gateway from existing Secrets or ConfigMaps (list of `secretRef`/`configMapRef` entries). Same shape as agent-level `envFrom`. | `[]` | | `cron.usercronEnabled` | Enable user-provided cron configuration. | `false` | | `cronjobs` | Config-driven scheduled messages for an agent. | `[]` | | `persistence.enabled` | Enable persistent storage for auth and settings. | `true` | diff --git a/charts/openab/templates/gateway.yaml b/charts/openab/templates/gateway.yaml index 2a89dc79a..026a69d38 100644 --- a/charts/openab/templates/gateway.yaml +++ b/charts/openab/templates/gateway.yaml @@ -220,6 +220,14 @@ spec: {{- end }} - name: RUST_LOG value: {{ ($cfg.gateway).rustLog | default "info" | quote }} + {{- range $k, $v := ($cfg.gateway).env }} + - name: {{ $k }} + value: {{ $v | quote }} + {{- end }} + {{- with ($cfg.gateway).envFrom }} + envFrom: + {{- toYaml . | nindent 12 }} + {{- end }} livenessProbe: httpGet: path: /health diff --git a/charts/openab/templates/networkpolicy.yaml b/charts/openab/templates/networkpolicy.yaml index 62ed10aa4..ca0463106 100644 --- a/charts/openab/templates/networkpolicy.yaml +++ b/charts/openab/templates/networkpolicy.yaml @@ -123,18 +123,22 @@ spec: ports: {{- toYaml $np.egress.proxy.ports | nindent 4 }} {{- else }} +{{- $mdxV4 := $np.egress.metadataExclusions.ipv4 | default (list "169.254.169.254/32") }} +{{- $mdxV6 := $np.egress.metadataExclusions.ipv6 | default (list "fd00:ec2::254/128") }} {{- if $np.egress.allowHttps }} - to: - ipBlock: cidr: 0.0.0.0/0 except: - - 169.254.169.254/32 + {{- toYaml $mdxV4 | nindent 10 }} {{- if $np.egress.allowIpv6 }} -{{/* F8: IPv6 IMDS excludes the AWS pattern; other clouds may need extraRules */}} +{{/* F8/F2: IPv6 exclusions configurable via metadataExclusions.ipv6 — + additive NetworkPolicy semantics mean extraRules cannot subtract + from an existing allow. */}} - ipBlock: cidr: ::/0 except: - - fd00:ec2::254/128 + {{- toYaml $mdxV6 | nindent 10 }} {{- end }} ports: - protocol: TCP @@ -145,12 +149,12 @@ spec: - ipBlock: cidr: 0.0.0.0/0 except: - - 169.254.169.254/32 + {{- toYaml $mdxV4 | nindent 10 }} {{- if $np.egress.allowIpv6 }} - ipBlock: cidr: ::/0 except: - - fd00:ec2::254/128 + {{- toYaml $mdxV6 | nindent 10 }} {{- end }} ports: - protocol: TCP diff --git a/charts/openab/tests/networkpolicy_test.yaml b/charts/openab/tests/networkpolicy_test.yaml index c6175bd36..384fa9afd 100644 --- a/charts/openab/tests/networkpolicy_test.yaml +++ b/charts/openab/tests/networkpolicy_test.yaml @@ -1,14 +1,17 @@ suite: NetworkPolicy support (opt-in, direct + proxy egress modes) templates: - templates/networkpolicy.yaml + - templates/gateway.yaml tests: - it: renders no NetworkPolicy resource when networkPolicy.enabled is false (default) + template: templates/networkpolicy.yaml asserts: - hasDocuments: count: 0 - it: renders a single agent NetworkPolicy in direct mode (gateway off) with deny-all ingress, DNS scoped to kube-system, and IPv4+IPv6 HTTPS egress + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true asserts: @@ -61,6 +64,7 @@ tests: port: 443 - it: renders both agent and gateway NetworkPolicies in direct mode when gateway is deployed, and the agent policy authorizes TCP 8080 to the gateway pod (F2) + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true agents.kiro.gateway.enabled: true @@ -101,6 +105,7 @@ tests: documentIndex: 0 - it: does NOT render a gateway NetworkPolicy or an agent→gateway egress rule when gateway.enabled=true but gateway.deploy=false (F3 regression — matches templates/gateway.yaml condition) + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true agents.kiro.gateway.enabled: true @@ -126,6 +131,7 @@ tests: any: true - it: renders HTTP egress rule (IPv4+IPv6) when egress.allowHttp is true (direct mode) + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true networkPolicy.egress.allowHttp: true @@ -147,6 +153,7 @@ tests: port: 80 - it: omits IPv6 ipBlock when allowIpv6=false (direct mode, IPv4-only cluster) (F8) + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true networkPolicy.egress.allowIpv6: false @@ -173,6 +180,7 @@ tests: any: true - it: falls back to any-namespace DNS when egress.dnsNamespace is empty string (F4 legacy escape hatch) + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true networkPolicy.egress.dnsNamespace: "" @@ -192,6 +200,7 @@ tests: port: 53 - it: restricts agent egress to the designated proxy Service in proxy mode (F5 — enforcement only) + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true networkPolicy.egress.mode: proxy @@ -232,6 +241,7 @@ tests: any: true - it: quotes an all-numeric proxy namespace so it renders as a string, not a YAML integer (F7) + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true networkPolicy.egress.mode: proxy @@ -246,6 +256,7 @@ tests: value: "1234" - it: applies proxy-mode egress to both agent and gateway policies when gateway is deployed + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true networkPolicy.egress.mode: proxy @@ -288,6 +299,7 @@ tests: documentIndex: 1 - it: appends ingress.extraRules verbatim to the agent policy + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true networkPolicy.ingress.extraRules: @@ -311,6 +323,7 @@ tests: port: 9090 - it: locks egress down to only extraRules when allowDns and allowHttps are false (direct mode) + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true networkPolicy.egress.allowDns: false @@ -360,6 +373,7 @@ tests: # ── F1 negative tests: fail-closed validation ───────────────────────────── - it: fails rendering when egress.mode is not exactly "direct" or "proxy" (F1) + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true networkPolicy.egress.mode: proxxy @@ -368,6 +382,7 @@ tests: errorMessage: 'networkPolicy.egress.mode must be exactly "direct" or "proxy" (got "proxxy")' - it: fails rendering when mode=proxy but proxy.podSelector is empty (F1) + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true networkPolicy.egress.mode: proxy @@ -379,6 +394,7 @@ tests: errorMessage: 'networkPolicy.egress.mode=proxy requires networkPolicy.egress.proxy.podSelector.matchLabels or matchExpressions to be non-empty (an empty selector authorizes every pod in the namespace)' - it: fails rendering when mode=proxy but proxy.ports is empty (F1) + template: templates/networkpolicy.yaml set: networkPolicy.enabled: true networkPolicy.egress.mode: proxy @@ -386,3 +402,75 @@ tests: asserts: - failedTemplate: errorMessage: 'networkPolicy.egress.mode=proxy requires networkPolicy.egress.proxy.ports to be non-empty (an empty ports list authorizes every port)' + + # ── Round-2 review fixes ───────────────────────────────────────────────── + + - it: injects gateway.env into the gateway Deployment so proxy-mode gateway can route outbound HTTPS through the proxy (round-2 F1) + template: templates/gateway.yaml + set: + agents.kiro.gateway.enabled: true + agents.kiro.gateway.env: + HTTPS_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" + HTTP_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" + NO_PROXY: ".svc,.cluster.local,10.0.0.0/8" + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: HTTPS_PROXY + value: "http://openshell.egress-proxy.svc.cluster.local:3128" + documentIndex: 0 + - contains: + path: spec.template.spec.containers[0].env + content: + name: HTTP_PROXY + value: "http://openshell.egress-proxy.svc.cluster.local:3128" + documentIndex: 0 + - contains: + path: spec.template.spec.containers[0].env + content: + name: NO_PROXY + value: ".svc,.cluster.local,10.0.0.0/8" + documentIndex: 0 + + - it: renders gateway.envFrom on the gateway Deployment (round-2 F1) + template: templates/gateway.yaml + set: + agents.kiro.gateway.enabled: true + agents.kiro.gateway.envFrom: + - secretRef: + name: proxy-credentials + asserts: + - equal: + path: spec.template.spec.containers[0].envFrom + value: + - secretRef: + name: proxy-credentials + documentIndex: 0 + + - it: uses configurable metadataExclusions for both IPv4 and IPv6 in direct-mode HTTPS rule (round-2 F2) + template: templates/networkpolicy.yaml + set: + networkPolicy.enabled: true + networkPolicy.egress.metadataExclusions.ipv4: + - 169.254.169.254/32 + - 169.254.170.2/32 + networkPolicy.egress.metadataExclusions.ipv6: + - fd00:beef::254/128 + asserts: + - contains: + path: spec.egress + content: + to: + - ipBlock: + cidr: 0.0.0.0/0 + except: + - 169.254.169.254/32 + - 169.254.170.2/32 + - ipBlock: + cidr: ::/0 + except: + - fd00:beef::254/128 + ports: + - protocol: TCP + port: 443 diff --git a/charts/openab/values.yaml b/charts/openab/values.yaml index 1d2a13e77..d20fd410e 100644 --- a/charts/openab/values.yaml +++ b/charts/openab/values.yaml @@ -78,42 +78,68 @@ networkPolicy: dnsNamespace: "kube-system" # ── mode: direct ──────────────────────────────────────────────────────── - # Allow HTTPS (TCP 443). Excludes cloud metadata service (169.254.169.254/32 - # for IPv4, fd00:ec2::254/128 for AWS IPv6) — blocks a common IAM-credential- - # theft path from a compromised agent pod. Ignored when mode=proxy. + # Allow HTTPS (TCP 443). Excludes cloud metadata (see metadataExclusions + # below) — blocks a common IAM-credential-theft path from a compromised + # agent pod. Ignored when mode=proxy. allowHttps: true # Allow HTTP (TCP 80) with the same metadata exclusion. Off by default. # ⚠️ Enable if any of these fetch over HTTP: `agents..configUrl`, # lifecycle hooks that download over HTTP (see docs/hooks.md), or STT # base URLs on HTTP. Prefer HTTPS wherever possible. Ignored when mode=proxy. allowHttp: false - # Allow IPv6 egress in direct mode by adding ::/0 alongside 0.0.0.0/0 in - # the HTTPS/HTTP rules, with AWS IMDS IPv6 (fd00:ec2::254/128) excluded. - # ⚠️ Non-AWS clouds use different IPv6 metadata addresses; override via - # extraRules if you need a tighter exclusion. Turn off on IPv4-only - # clusters if you prefer explicit denial to allowing unused v6 ranges. + # Add ::/0 to HTTPS/HTTP rules alongside 0.0.0.0/0 for IPv6 egress. + # Turn off on IPv4-only clusters if you prefer explicit denial to + # allowing unused v6 ranges. Ignored when mode=proxy. allowIpv6: true + # Cloud metadata CIDRs excluded from the direct-mode HTTPS/HTTP rules. + # NetworkPolicy allow rules combine additively — you CANNOT add another + # exclusion via extraRules; you must edit these lists directly (or set + # allowHttps=false + supply a complete replacement rule via extraRules). + # + # Defaults cover AWS (169.254.169.254 works for GCP and Azure too; + # IPv6 IMDS varies by cloud). + # AWS IPv6 IMDS: fd00:ec2::254/128 + # GCP: fd00::/8 metadata routing varies — check your setup + # Azure: IPv4 only (169.254.169.254) + # IBM Cloud: 161.26.0.0/16 (IPv4) + metadataExclusions: + ipv4: + - 169.254.169.254/32 + ipv6: + - fd00:ec2::254/128 # ── mode: proxy ───────────────────────────────────────────────────────── # ⚠️ Proxy-mode CONTRACT # NetworkPolicy only ENFORCES the destination allowlist. It does NOT - # redirect traffic through the proxy. You must configure your pods to - # actually send traffic through the proxy — one common way is env vars: + # redirect traffic through the proxy. You must configure BOTH the agent + # AND the chart-managed gateway (if deployed) to route their outbound + # traffic through the proxy — otherwise the NetworkPolicy deny will + # break platform API calls (Telegram replies, Teams OAuth, LLM APIs). + # + # 1. Agent — set proxy env vars via agents..env AND add them to + # inherit_env in configToml so ACP children (claude-agent-acp, + # kiro-cli, opencode, etc.) inherit them: + # + # agents..env: + # HTTPS_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" + # HTTP_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" + # NO_PROXY: ".svc,.cluster.local,10.0.0.0/8" # - # agents..env: - # HTTPS_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" - # HTTP_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" - # NO_PROXY: ".svc,.cluster.local,10.0.0.0/8" + # agents..configToml: | + # [agent] + # inherit_env = ["HTTPS_PROXY","HTTP_PROXY","NO_PROXY"] # - # And add these variables to inherit_env in your configToml so ACP children - # (e.g. claude-agent-acp, kiro-cli, opencode) inherit the proxy settings: + # 2. Gateway — the gateway pod ALSO makes outbound HTTPS calls (Telegram + # replies, Teams OAuth token exchange, Feishu/WeCom APIs, etc.). Set + # the same env vars on it via the new agents..gateway.env field: # - # [agent] - # inherit_env = ["HTTPS_PROXY","HTTP_PROXY","NO_PROXY"] + # agents..gateway.env: + # HTTPS_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" + # HTTP_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" + # NO_PROXY: ".svc,.cluster.local,10.0.0.0/8" # - # Without both pieces, agent traffic will hit the NetworkPolicy egress deny - # and fail to connect. Chart does NOT ship the proxy — deploy it separately - # (OpenSHELL, Squid, Envoy, mitmproxy, or anything speaking HTTP CONNECT). + # Chart does NOT ship the proxy — deploy it separately (OpenSHELL, + # Squid, Envoy, mitmproxy, or anything speaking HTTP CONNECT). # # egress.allowDns above still applies. allowHttps/allowHttp/allowIpv6 are # IGNORED when mode=proxy. @@ -496,6 +522,19 @@ agents: strategy: "Recreate" # Recreate (default, prevents concurrent WS conflicts) or RollingUpdate resources: {} # e.g. { requests: { cpu: 50m, memory: 64Mi }, limits: { memory: 128Mi } } rustLog: "info" # RUST_LOG for gateway container (e.g. "openab_gateway=debug") + # Additional environment variables for the gateway container. + # Useful for injecting HTTPS_PROXY / HTTP_PROXY / NO_PROXY when running + # under networkPolicy.egress.mode=proxy (the gateway also makes outbound + # HTTPS calls to platform APIs — Telegram replies, Teams OAuth, etc.). + # Example: + # env: + # HTTPS_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" + # HTTP_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" + # NO_PROXY: ".svc,.cluster.local,10.0.0.0/8" + env: {} + # Load additional env vars into the gateway from existing Secrets or ConfigMaps. + # Same shape as agent-level `envFrom` (list of secretRef/configMapRef entries). + envFrom: [] nodeSelector: {} tolerations: [] affinity: {} From a66f3aae35f6ade4aca3b9ec1aca18526b690eab Mon Sep 17 00:00:00 2001 From: yen <5915590+antigenius0910@users.noreply.github.com> Date: Tue, 14 Jul 2026 19:18:54 -0500 Subject: [PATCH 4/8] =?UTF-8?q?fix(gateway,chart):=20address=20PR=20#1400?= =?UTF-8?q?=20round-3=20review=20=E2=80=94=20reqwest=20system-proxy=20+=20?= =?UTF-8?q?CNI/IMDS=20caveats?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all three round-3 findings from chaodu-agent on commit f323386. Kept as a fourth commit rather than amend to preserve review history. Critical (round-3 F1) — Enable reqwest system-proxy in openab-gateway: The chart correctly renders HTTPS_PROXY / HTTP_PROXY / NO_PROXY into the gateway container env (round-2 fix), but the gateway binary was compiled with `reqwest = { default-features = false, features = ["rustls-tls", "json"] }`. In reqwest 0.12, `system-proxy` is a *default* feature that turns on env-var auto-detection (HTTP_PROXY / HTTPS_PROXY / NO_PROXY / ALL_PROXY). With `default-features = false` and `system-proxy` not re-enabled, reqwest silently ignores those env vars. Effect: proxy-mode gateway pods were reachable to the proxy Service and DNS, but every outbound platform-API call (Telegram replies, Teams OAuth token exchange, Feishu/WeCom/Google Chat/LINE callouts, WebSocket downstream setup) tried direct egress and hit the NetworkPolicy deny. Fix: add `"system-proxy"` to reqwest's feature list in crates/openab-gateway/Cargo.toml. Verified with `cargo check -p openab-gateway` (clean build) and `cargo test -p openab-gateway --lib` (262 tests pass, no regression). Added an inline comment above the dependency line explaining why the feature is re-enabled — otherwise a future developer might strip it during a dependency cleanup. Rust code is unchanged. All existing HTTP clients constructed via `reqwest::Client::new()` or `Client::builder()` now respect env-var proxy config automatically. Important (round-3 F2) — Document CNI-enforcement prerequisite: A NetworkPolicy has no effect unless the cluster's CNI enforces it — stock KIND, Docker Desktop, or bare-EKS clusters without a supported CNI silently ignore the resource. Added an explicit "⚠️ PREREQUISITE" block at the top of the values.yaml networkPolicy section listing supported CNIs (Calico, Cilium, Antrea, EKS with NetworkPolicy add-on, GKE Dataplane V2, AKS with Azure NPM / Calico) and recommending a scratch-namespace deny-all verification before relying on the feature. Mirrored the caveat into the chart README row for networkPolicy.enabled. Important (round-3 F3) — Qualify metadata isolation as defense-in-depth: The metadataExclusions rendering is correct under additive NetworkPolicy semantics (round-2 fix), but the docs presented it as portable IMDS protection. K8s NetworkPolicy always permits pod-to-resident-node traffic, and CNI implementations differ in whether ipBlock matches pre- or post-NAT addresses. Added an explicit "⚠️ Treat this as defense-in- depth, NOT a portable IMDS guarantee" block in values.yaml enumerating the two K8s-level caveats and recommending cloud-provider hardening (IMDSv2 mandatory + hop-limit 1, GCP metadata concealment, Azure metadata endpoint policy) as the primary control. Mirrored into the chart README rows for both metadataExclusions.ipv4 and .ipv6. No behavioural change beyond the reqwest feature addition — F2 and F3 are documentation-only. Existing 58 helm-unittest tests still pass. Refs #1394 Refs #1400 round-3 review by chaodu-agent --- Cargo.lock | 47 +++++++++++++++++++++++++++++++- charts/openab/README.md | 6 ++-- charts/openab/values.yaml | 20 ++++++++++++++ crates/openab-gateway/Cargo.toml | 6 +++- 4 files changed, 74 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 74ddee92f..aa3028498 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -959,6 +959,16 @@ version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a6ef517f0926dd24a1582492c791b6a4818a4d94e789a334894aa15b0d12f55c" +[[package]] +name = "core-foundation" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91e195e091a93c46f7102ec7818a2aa394e1e1771c3ab4825963fa03e45afb8f" +dependencies = [ + "core-foundation-sys", + "libc", +] + [[package]] name = "core-foundation" version = "0.10.1" @@ -1750,9 +1760,11 @@ dependencies = [ "percent-encoding", "pin-project-lite", "socket2 0.6.4", + "system-configuration", "tokio", "tower-service", "tracing", + "windows-registry", ] [[package]] @@ -2222,6 +2234,7 @@ dependencies = [ "aws-sdk-secretsmanager", "aws-sigv4", "base64", + "bytes", "chrono", "chrono-tz", "clap", @@ -3032,7 +3045,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b7f4bc775c73d9a02cde8bf7b2ec4c9d12743edf609006c7facc23998404cd1d" dependencies = [ "bitflags", - "core-foundation", + "core-foundation 0.10.1", "core-foundation-sys", "libc", "security-framework-sys", @@ -3369,6 +3382,27 @@ dependencies = [ "syn", ] +[[package]] +name = "system-configuration" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a13f3d0daba03132c0aa9767f98351b3488edc2c100cda2d2ec2b04f3d8d3c8b" +dependencies = [ + "bitflags", + "core-foundation 0.9.4", + "system-configuration-sys", +] + +[[package]] +name = "system-configuration-sys" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e1d1b10ced5ca923a1fcb8d03e96b8d3268065d724548c0211415ff6ac6bac4" +dependencies = [ + "core-foundation-sys", + "libc", +] + [[package]] name = "tar" version = "0.4.46" @@ -4080,6 +4114,17 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" +[[package]] +name = "windows-registry" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "02752bf7fbdcce7f2a27a742f798510f3e5ad88dbe84871e5168e2120c3d5720" +dependencies = [ + "windows-link", + "windows-result", + "windows-strings", +] + [[package]] name = "windows-result" version = "0.4.1" diff --git a/charts/openab/README.md b/charts/openab/README.md index 3845a5ed0..cc7b06241 100644 --- a/charts/openab/README.md +++ b/charts/openab/README.md @@ -14,7 +14,7 @@ This page highlights commonly used values and deployment patterns. For the compl | `fullnameOverride` | Override the full generated release name for chart resources. Useful when deploying multiple instances with predictable names. | `""` | | `serviceAccountName` | Chart-global ServiceAccount name attached to every agent pod that doesn't define its own. Empty = cluster `default` SA. Per-agent `agents..serviceAccountName` fully overrides this. Chart references an existing SA only — does not create one. Required for workload identity and pod-level RBAC. | `""` | | `imagePullSecrets` | Chart-global image pull secrets attached to every agent pod that doesn't define its own. Per-agent `agents..imagePullSecrets` fully overrides this. | `[]` | -| `networkPolicy.enabled` | Master switch for the chart-managed NetworkPolicy. Off by default — no `NetworkPolicy` resource is rendered until this flips to `true`, keeping existing releases unaffected. When on: one policy per agent (deny-all ingress + configured egress + TCP 8080 to the chart-managed gateway pod when `gateway.enabled=true` and `gateway.deploy` is not `false`), plus one policy per deployed gateway (port 8080 ingress + same egress). | `false` | +| `networkPolicy.enabled` | Master switch for the chart-managed NetworkPolicy. Off by default — no `NetworkPolicy` resource is rendered until this flips to `true`, keeping existing releases unaffected. When on: one policy per agent (deny-all ingress + configured egress + TCP 8080 to the chart-managed gateway pod when `gateway.enabled=true` and `gateway.deploy` is not `false`), plus one policy per deployed gateway (port 8080 ingress + same egress). ⚠️ **Prerequisite: your cluster's CNI must enforce NetworkPolicy** (Calico, Cilium, Antrea, EKS with NetworkPolicy add-on, GKE with Dataplane V2 / Calico, AKS with Azure NPM / Calico, etc.). Stock KIND, Docker Desktop, or bare-EKS clusters without a NetworkPolicy-capable CNI silently ignore the resource — verify enforcement with a scratch namespace + deny-all test before relying on this. | `false` | | `networkPolicy.ingress.extraRules` | Raw `NetworkPolicyIngressRule` entries appended verbatim to every rendered policy. Use for cluster-specific allow-lists (Prometheus scrape, mesh sidecars, etc.). | `[]` | | `networkPolicy.egress.mode` | Egress topology. Must be exactly `"direct"` or `"proxy"` — invalid values fail the render (fail-closed). `direct` allows egress to the open internet (DNS + HTTPS by default). `proxy` restricts egress to a designated proxy Service — recommended for enterprise / multi-tenant / compliance-driven deployments (see [RFC #1394](https://github.com/openabdev/openab/issues/1394) and OpenSHELL prior art). **Proxy mode enforces the destination allowlist only; you must also configure your workloads to send traffic through the proxy: set `HTTPS_PROXY` / `HTTP_PROXY` on the agent via `agents..env` + `inherit_env` in `configToml`, AND on the gateway via `agents..gateway.env` (the gateway makes its own outbound HTTPS calls to platform APIs).** Chart does not ship the proxy — BYO (OpenSHELL / Squid / Envoy / mitmproxy). | `"direct"` | | `networkPolicy.egress.allowDns` | Allow DNS egress on UDP+TCP 53 to pods labelled `k8s-app=kube-dns`. Applies in both `direct` and `proxy` modes. | `true` | @@ -22,8 +22,8 @@ This page highlights commonly used values and deployment patterns. For the compl | `networkPolicy.egress.allowHttps` | (direct mode only) Allow HTTPS (TCP 443) to `0.0.0.0/0` (and `::/0` when `allowIpv6=true`), except CIDRs listed in `metadataExclusions`. Ignored in proxy mode. | `true` | | `networkPolicy.egress.allowHttp` | (direct mode only) Allow HTTP (TCP 80) with the same metadata exclusion. Off by default. ⚠️ Enable if any of these fetch over HTTP: `agents..configUrl`, lifecycle hooks that download over HTTP (see `docs/hooks.md`), or STT base URLs on HTTP. Prefer HTTPS wherever possible. Ignored in proxy mode. | `false` | | `networkPolicy.egress.allowIpv6` | (direct mode only) Add `::/0` to the HTTPS/HTTP rules alongside `0.0.0.0/0`, excluding CIDRs from `metadataExclusions.ipv6`. Turn off on IPv4-only clusters. Ignored in proxy mode. | `true` | -| `networkPolicy.egress.metadataExclusions.ipv4` | CIDRs excluded from the direct-mode IPv4 HTTPS/HTTP allow rule. Defaults to `169.254.169.254/32` (AWS/GCP/Azure IMDS). Add IBM Cloud's `161.26.0.0/16` here if applicable. **NetworkPolicy allow rules combine additively — you cannot subtract via `extraRules`; edit this list directly.** | `["169.254.169.254/32"]` | -| `networkPolicy.egress.metadataExclusions.ipv6` | CIDRs excluded from the direct-mode IPv6 HTTPS/HTTP allow rule. Defaults to `fd00:ec2::254/128` (AWS IPv6 IMDS). Non-AWS IPv6 metadata addresses differ — override this list per cloud. **Same additive-only semantics as `ipv4` above.** | `["fd00:ec2::254/128"]` | +| `networkPolicy.egress.metadataExclusions.ipv4` | CIDRs excluded from the direct-mode IPv4 HTTPS/HTTP allow rule. Defaults to `169.254.169.254/32` (AWS/GCP/Azure IMDS). Add IBM Cloud's `161.26.0.0/16` here if applicable. **NetworkPolicy allow rules combine additively — you cannot subtract via `extraRules`; edit this list directly.** ⚠️ **Defense-in-depth only, not a portable IMDS guarantee**: pods can always reach services on their resident node, and CNI implementations differ in whether `ipBlock` matches pre- or post-NAT addresses. Use cloud-provider hardening (IMDSv2 mandatory + hop-limit 1, GCP metadata concealment, Azure metadata endpoint policy) as the primary control. | `["169.254.169.254/32"]` | +| `networkPolicy.egress.metadataExclusions.ipv6` | CIDRs excluded from the direct-mode IPv6 HTTPS/HTTP allow rule. Defaults to `fd00:ec2::254/128` (AWS IPv6 IMDS). Non-AWS IPv6 metadata addresses differ — override this list per cloud. **Same additive-only semantics as `ipv4` above, and same defense-in-depth caveat.** | `["fd00:ec2::254/128"]` | | `networkPolicy.egress.proxy` | (proxy mode only) Selector for the egress proxy Service — `{namespace, podSelector, ports}`. Render fails if `podSelector` is empty or `ports` is empty (would otherwise authorize every pod on every port — fail-open). Deploy the proxy separately. | `{namespace: "", podSelector: {}, ports: []}` | | `networkPolicy.egress.extraRules` | Raw `NetworkPolicyEgressRule` entries appended verbatim. Use to open extra ports or add cluster-specific carve-outs. **Cannot subtract from existing allows — for tighter metadata exclusions use `metadataExclusions` above.** | `[]` | diff --git a/charts/openab/values.yaml b/charts/openab/values.yaml index d20fd410e..963aaabd1 100644 --- a/charts/openab/values.yaml +++ b/charts/openab/values.yaml @@ -46,6 +46,15 @@ containerSecurityContext: # NetworkPolicy for pod-level network isolation. Opt-in and backwards-compatible: # no NetworkPolicy resource is rendered unless networkPolicy.enabled=true. # See RFC: https://github.com/openabdev/openab/issues/1394 +# +# ⚠️ PREREQUISITE — CNI enforcement +# A NetworkPolicy has NO EFFECT unless the cluster's CNI enforces it. Verify +# your cluster runs a supported CNI (Calico, Cilium, Antrea, EKS with the +# VPC CNI + NetworkPolicy add-on, GKE with Dataplane V2 or Calico, AKS with +# Azure NPM or Calico, etc.). On stock KIND, Docker Desktop, or a bare-EKS +# cluster without NetworkPolicy support, enabling this flag renders a +# resource that the CNI simply ignores — you get a false sense of isolation. +# Test with a deny-all policy in a scratch namespace before relying on it. networkPolicy: enabled: false @@ -96,6 +105,17 @@ networkPolicy: # exclusion via extraRules; you must edit these lists directly (or set # allowHttps=false + supply a complete replacement rule via extraRules). # + # ⚠️ Treat this as defense-in-depth, NOT a portable IMDS guarantee: + # - Kubernetes NetworkPolicy always permits traffic between a pod and + # its own resident node — a compromised pod scheduled on the node + # running metadata-service DNAT can still reach IMDS. + # - The order of address rewriting (SNAT/DNAT) vs. `ipBlock` matching + # is CNI-implementation-dependent — some CNIs match on pre-NAT + # addresses, others on post-NAT. + # - Cloud-provider hardening (IMDSv2 mandatory / hop-limit 1, disable + # IMDSv1, GCP metadata concealment, Azure metadata endpoint policy) + # is the primary control. This exclusion is a secondary net. + # # Defaults cover AWS (169.254.169.254 works for GCP and Azure too; # IPv6 IMDS varies by cloud). # AWS IPv6 IMDS: fd00:ec2::254/128 diff --git a/crates/openab-gateway/Cargo.toml b/crates/openab-gateway/Cargo.toml index 26ee00db9..f7854095e 100644 --- a/crates/openab-gateway/Cargo.toml +++ b/crates/openab-gateway/Cargo.toml @@ -11,7 +11,11 @@ tokio-tungstenite = { version = "0.21", features = ["rustls-tls-webpki-roots"] } futures-util = "0.3" serde = { version = "1", features = ["derive"] } serde_json = "1" -reqwest = { version = "0.12", default-features = false, features = ["rustls-tls", "json"] } +# `system-proxy` re-enables HTTPS_PROXY / HTTP_PROXY / NO_PROXY env-var auto-detection, +# which is a default reqwest feature disabled by `default-features = false`. Required so +# the gateway respects proxy env vars injected via chart `agents..gateway.env` when +# running under `networkPolicy.egress.mode=proxy`. See PR #1400. +reqwest = { version = "0.12", default-features = false, features = ["rustls-tls", "json", "system-proxy"] } tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } anyhow = "1" From b0a35dd3e79cdf326f5651e0a2763735aca9e58c Mon Sep 17 00:00:00 2001 From: yen <5915590+antigenius0910@users.noreply.github.com> Date: Tue, 14 Jul 2026 20:19:37 -0500 Subject: [PATCH 5/8] =?UTF-8?q?fix(chart):=20address=20PR=20#1400=20round-?= =?UTF-8?q?4=20review=20=E2=80=94=20Feishu=20WSS=20incompatibility,=20stri?= =?UTF-8?q?cter=20mode/port=20validation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all three round-4 findings from chaodu-agent on commit a66f3aa. Kept as a fifth commit rather than amend to preserve review history. Critical (round-4 F1) — Feishu WebSocket + proxy-mode incompatibility: The reqwest system-proxy fix (round-3) makes gateway HTTP clients honor HTTPS_PROXY, but Feishu's default connectionMode="websocket" opens its event stream via tokio_tungstenite::connect_async which does NOT consume reqwest's proxy configuration. Under networkPolicy.egress.mode=proxy the WSS TCP 443 direct-egress attempt is blocked — Feishu gateway can't establish its event connection. Rather than implement a full HTTP CONNECT-aware WebSocket path (large Rust scope), enforce the constraint at chart render time with a clear error pointing operators at the supported fallback: fail render if networkPolicy.egress.mode=proxy AND any agent has gateway.feishu.appId set AND gateway.feishu.connectionMode is unset or "websocket". Positive path (connectionMode="webhook") renders cleanly. Documented in values.yaml and the chart README row for networkPolicy.egress.mode. Critical (round-4 F2) — Empty-string mode silently defaulted to direct: The previous `$mode := $np.egress.mode | default "direct"` treated an explicit `--set-string networkPolicy.egress.mode=""` as "direct" and rendered open-internet HTTPS, contradicting the fail-closed contract. Fix: drop the `| default "direct"` — validate the raw value directly. values.yaml already provides the default; explicitly-empty input is now rejected with the same error format as any other invalid mode value. Important (round-4 F3) — Protocol-only proxy port entries: The previous check ensured proxy.ports was non-empty, but a valid NetworkPolicyPort like `{protocol: TCP}` (no `port` field) passed and rendered — Kubernetes treats missing `port` as "all ports for the given protocol", silently broadening egress to the proxy pods. Fix: iterate proxy.ports and require every entry to have a truthy `port` field; error message names the offending index. Documented in the chart README row for networkPolicy.egress.proxy. Tests: 4 new helm-unittest cases (62 total, was 58): - F2: empty mode string → fail-closed with exact error message - F3: proxy port entry with only `protocol` → fail-closed - F1: proxy+feishu websocket → fail-closed with actionable message - F1 positive: proxy+feishu webhook → 2 policies render as expected Full chart suite: 62/62 pass. helm lint clean. No Rust changes. Refs #1394 Refs #1400 round-4 review by chaodu-agent --- charts/openab/README.md | 4 +- charts/openab/templates/networkpolicy.yaml | 29 ++++++++++- charts/openab/tests/networkpolicy_test.yaml | 56 +++++++++++++++++++++ charts/openab/values.yaml | 18 ++++++- 4 files changed, 101 insertions(+), 6 deletions(-) diff --git a/charts/openab/README.md b/charts/openab/README.md index cc7b06241..3b1233951 100644 --- a/charts/openab/README.md +++ b/charts/openab/README.md @@ -16,7 +16,7 @@ This page highlights commonly used values and deployment patterns. For the compl | `imagePullSecrets` | Chart-global image pull secrets attached to every agent pod that doesn't define its own. Per-agent `agents..imagePullSecrets` fully overrides this. | `[]` | | `networkPolicy.enabled` | Master switch for the chart-managed NetworkPolicy. Off by default — no `NetworkPolicy` resource is rendered until this flips to `true`, keeping existing releases unaffected. When on: one policy per agent (deny-all ingress + configured egress + TCP 8080 to the chart-managed gateway pod when `gateway.enabled=true` and `gateway.deploy` is not `false`), plus one policy per deployed gateway (port 8080 ingress + same egress). ⚠️ **Prerequisite: your cluster's CNI must enforce NetworkPolicy** (Calico, Cilium, Antrea, EKS with NetworkPolicy add-on, GKE with Dataplane V2 / Calico, AKS with Azure NPM / Calico, etc.). Stock KIND, Docker Desktop, or bare-EKS clusters without a NetworkPolicy-capable CNI silently ignore the resource — verify enforcement with a scratch namespace + deny-all test before relying on this. | `false` | | `networkPolicy.ingress.extraRules` | Raw `NetworkPolicyIngressRule` entries appended verbatim to every rendered policy. Use for cluster-specific allow-lists (Prometheus scrape, mesh sidecars, etc.). | `[]` | -| `networkPolicy.egress.mode` | Egress topology. Must be exactly `"direct"` or `"proxy"` — invalid values fail the render (fail-closed). `direct` allows egress to the open internet (DNS + HTTPS by default). `proxy` restricts egress to a designated proxy Service — recommended for enterprise / multi-tenant / compliance-driven deployments (see [RFC #1394](https://github.com/openabdev/openab/issues/1394) and OpenSHELL prior art). **Proxy mode enforces the destination allowlist only; you must also configure your workloads to send traffic through the proxy: set `HTTPS_PROXY` / `HTTP_PROXY` on the agent via `agents..env` + `inherit_env` in `configToml`, AND on the gateway via `agents..gateway.env` (the gateway makes its own outbound HTTPS calls to platform APIs).** Chart does not ship the proxy — BYO (OpenSHELL / Squid / Envoy / mitmproxy). | `"direct"` | +| `networkPolicy.egress.mode` | Egress topology. Must be exactly `"direct"` or `"proxy"` — anything else (including an empty string) fails the render (fail-closed). `direct` allows egress to the open internet (DNS + HTTPS by default). `proxy` restricts egress to a designated proxy Service — recommended for enterprise / multi-tenant / compliance-driven deployments (see [RFC #1394](https://github.com/openabdev/openab/issues/1394) and OpenSHELL prior art). **Proxy mode enforces the destination allowlist only; you must also configure your workloads to send traffic through the proxy: set `HTTPS_PROXY` / `HTTP_PROXY` on the agent via `agents..env` + `inherit_env` in `configToml`, AND on the gateway via `agents..gateway.env` (the gateway makes its own outbound HTTPS calls to platform APIs).** ⚠️ **Feishu WebSocket mode is incompatible with proxy egress** — the WSS connection bypasses reqwest and does not honor HTTP CONNECT proxies; the chart will fail render unless you set `gateway.feishu.connectionMode: "webhook"`. Chart does not ship the proxy — BYO (OpenSHELL / Squid / Envoy / mitmproxy). | `"direct"` | | `networkPolicy.egress.allowDns` | Allow DNS egress on UDP+TCP 53 to pods labelled `k8s-app=kube-dns`. Applies in both `direct` and `proxy` modes. | `true` | | `networkPolicy.egress.dnsNamespace` | Trusted namespace hosting the DNS pods. Default `"kube-system"` matches kubeadm / CoreDNS / kube-dns defaults and prevents a co-tenant labelling their own pod `k8s-app=kube-dns` from becoming an approved port-53 destination. Empty string reverts to the legacy any-namespace selector — use only if your DNS runs outside kube-system. | `"kube-system"` | | `networkPolicy.egress.allowHttps` | (direct mode only) Allow HTTPS (TCP 443) to `0.0.0.0/0` (and `::/0` when `allowIpv6=true`), except CIDRs listed in `metadataExclusions`. Ignored in proxy mode. | `true` | @@ -24,7 +24,7 @@ This page highlights commonly used values and deployment patterns. For the compl | `networkPolicy.egress.allowIpv6` | (direct mode only) Add `::/0` to the HTTPS/HTTP rules alongside `0.0.0.0/0`, excluding CIDRs from `metadataExclusions.ipv6`. Turn off on IPv4-only clusters. Ignored in proxy mode. | `true` | | `networkPolicy.egress.metadataExclusions.ipv4` | CIDRs excluded from the direct-mode IPv4 HTTPS/HTTP allow rule. Defaults to `169.254.169.254/32` (AWS/GCP/Azure IMDS). Add IBM Cloud's `161.26.0.0/16` here if applicable. **NetworkPolicy allow rules combine additively — you cannot subtract via `extraRules`; edit this list directly.** ⚠️ **Defense-in-depth only, not a portable IMDS guarantee**: pods can always reach services on their resident node, and CNI implementations differ in whether `ipBlock` matches pre- or post-NAT addresses. Use cloud-provider hardening (IMDSv2 mandatory + hop-limit 1, GCP metadata concealment, Azure metadata endpoint policy) as the primary control. | `["169.254.169.254/32"]` | | `networkPolicy.egress.metadataExclusions.ipv6` | CIDRs excluded from the direct-mode IPv6 HTTPS/HTTP allow rule. Defaults to `fd00:ec2::254/128` (AWS IPv6 IMDS). Non-AWS IPv6 metadata addresses differ — override this list per cloud. **Same additive-only semantics as `ipv4` above, and same defense-in-depth caveat.** | `["fd00:ec2::254/128"]` | -| `networkPolicy.egress.proxy` | (proxy mode only) Selector for the egress proxy Service — `{namespace, podSelector, ports}`. Render fails if `podSelector` is empty or `ports` is empty (would otherwise authorize every pod on every port — fail-open). Deploy the proxy separately. | `{namespace: "", podSelector: {}, ports: []}` | +| `networkPolicy.egress.proxy` | (proxy mode only) Selector for the egress proxy Service — `{namespace, podSelector, ports}`. Render fails if `podSelector` is empty, `ports` is empty, **or any port entry omits `port`** (Kubernetes treats a missing `port` as "all ports for the protocol" — fail-open). Deploy the proxy separately. | `{namespace: "", podSelector: {}, ports: []}` | | `networkPolicy.egress.extraRules` | Raw `NetworkPolicyEgressRule` entries appended verbatim. Use to open extra ports or add cluster-specific carve-outs. **Cannot subtract from existing allows — for tighter metadata exclusions use `metadataExclusions` above.** | `[]` | ### Agent values diff --git a/charts/openab/templates/networkpolicy.yaml b/charts/openab/templates/networkpolicy.yaml index ca0463106..615665dc5 100644 --- a/charts/openab/templates/networkpolicy.yaml +++ b/charts/openab/templates/networkpolicy.yaml @@ -1,7 +1,10 @@ {{- if .Values.networkPolicy.enabled }} {{- $np := .Values.networkPolicy }} -{{/* ---------- F1: mode and proxy-required-fields validation ---------- */}} -{{- $mode := $np.egress.mode | default "direct" }} +{{/* ---------- mode and proxy-required-fields validation ---------- */}} +{{/* F2: validate the raw value — do NOT default here. values.yaml already + supplies a default; treating an explicitly empty string as `direct` + silently bypasses the fail-closed contract. */}} +{{- $mode := $np.egress.mode }} {{- if not (or (eq $mode "direct") (eq $mode "proxy")) }} {{- fail (printf "networkPolicy.egress.mode must be exactly \"direct\" or \"proxy\" (got %q)" $mode) }} {{- end }} @@ -12,6 +15,28 @@ {{- if not $np.egress.proxy.ports }} {{- fail "networkPolicy.egress.mode=proxy requires networkPolicy.egress.proxy.ports to be non-empty (an empty ports list authorizes every port)" }} {{- end }} +{{/* F3: every proxy port entry must specify `port`. In a NetworkPolicyPort, + omitting `port` matches all ports for the given protocol, silently + broadening egress to the proxy pods beyond the intended target. */}} +{{- range $i, $p := $np.egress.proxy.ports }} +{{- if not $p.port }} +{{- fail (printf "networkPolicy.egress.mode=proxy requires every networkPolicy.egress.proxy.ports entry to specify port (entry index %d missing port — omitted port matches ALL ports for the given protocol)" $i) }} +{{- end }} +{{- end }} +{{/* F1: Feishu WebSocket mode is incompatible with proxy-only egress — + tokio_tungstenite::connect_async opens WSS directly and does not + consume HTTP CONNECT proxy configuration. Fail render early with a + clear pointer to the webhook fallback. */}} +{{- range $name, $cfg := .Values.agents }} +{{- if ne (include "openab.agentEnabled" $cfg) "false" }} +{{- if and (($cfg.gateway).enabled) (($cfg.gateway).feishu).appId }} +{{- $feishuMode := (($cfg.gateway).feishu).connectionMode | default "websocket" }} +{{- if eq $feishuMode "websocket" }} +{{- fail (printf "networkPolicy.egress.mode=proxy is incompatible with Feishu WebSocket mode for agent %q: the WSS event connection opens TCP 443 directly via tokio_tungstenite and does not honor HTTP CONNECT proxies. Set agents.%s.gateway.feishu.connectionMode=\"webhook\", or disable networkPolicy for this release." $name $name) }} +{{- end }} +{{- end }} +{{- end }} +{{- end }} {{- end }} {{- range $name, $cfg := .Values.agents }} {{- if ne (include "openab.agentEnabled" $cfg) "false" }} diff --git a/charts/openab/tests/networkpolicy_test.yaml b/charts/openab/tests/networkpolicy_test.yaml index 384fa9afd..0d34510eb 100644 --- a/charts/openab/tests/networkpolicy_test.yaml +++ b/charts/openab/tests/networkpolicy_test.yaml @@ -448,6 +448,62 @@ tests: name: proxy-credentials documentIndex: 0 + # ── Round-4 review fixes: fail-closed validation gaps ────────────────── + + - it: fails rendering when egress.mode is an explicitly empty string (round-4 F2) + template: templates/networkpolicy.yaml + set: + networkPolicy.enabled: true + networkPolicy.egress.mode: "" + asserts: + - failedTemplate: + errorMessage: 'networkPolicy.egress.mode must be exactly "direct" or "proxy" (got "")' + + - it: fails rendering when any proxy.ports entry omits `port` (round-4 F3) + template: templates/networkpolicy.yaml + set: + networkPolicy.enabled: true + networkPolicy.egress.mode: proxy + networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell + networkPolicy.egress.proxy.ports: + - protocol: TCP + asserts: + - failedTemplate: + errorMessage: 'networkPolicy.egress.mode=proxy requires every networkPolicy.egress.proxy.ports entry to specify port (entry index 0 missing port — omitted port matches ALL ports for the given protocol)' + + - it: fails rendering when proxy mode is combined with Feishu WebSocket mode (round-4 F1) + template: templates/networkpolicy.yaml + set: + networkPolicy.enabled: true + networkPolicy.egress.mode: proxy + networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell + networkPolicy.egress.proxy.ports: + - protocol: TCP + port: 3128 + agents.kiro.gateway.enabled: true + agents.kiro.gateway.feishu.appId: cli_test + agents.kiro.gateway.feishu.appSecret: secret + asserts: + - failedTemplate: + errorMessage: 'networkPolicy.egress.mode=proxy is incompatible with Feishu WebSocket mode for agent "kiro": the WSS event connection opens TCP 443 directly via tokio_tungstenite and does not honor HTTP CONNECT proxies. Set agents.kiro.gateway.feishu.connectionMode="webhook", or disable networkPolicy for this release.' + + - it: renders successfully when proxy mode is combined with Feishu WEBHOOK mode (round-4 F1 positive case) + template: templates/networkpolicy.yaml + set: + networkPolicy.enabled: true + networkPolicy.egress.mode: proxy + networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell + networkPolicy.egress.proxy.ports: + - protocol: TCP + port: 3128 + agents.kiro.gateway.enabled: true + agents.kiro.gateway.feishu.appId: cli_test + agents.kiro.gateway.feishu.appSecret: secret + agents.kiro.gateway.feishu.connectionMode: webhook + asserts: + - hasDocuments: + count: 2 + - it: uses configurable metadataExclusions for both IPv4 and IPv6 in direct-mode HTTPS rule (round-2 F2) template: templates/networkpolicy.yaml set: diff --git a/charts/openab/values.yaml b/charts/openab/values.yaml index 963aaabd1..59b408269 100644 --- a/charts/openab/values.yaml +++ b/charts/openab/values.yaml @@ -161,11 +161,25 @@ networkPolicy: # Chart does NOT ship the proxy — deploy it separately (OpenSHELL, # Squid, Envoy, mitmproxy, or anything speaking HTTP CONNECT). # + # ⚠️ Feishu WebSocket mode is INCOMPATIBLE with proxy egress. + # The Feishu adapter opens its event stream via + # tokio_tungstenite::connect_async, which does NOT honor HTTP CONNECT + # proxy env vars — even with reqwest's system-proxy feature enabled on + # the gateway. In proxy mode, the WSS connection tries direct TCP 443 + # to the Feishu endpoint and is blocked by the NetworkPolicy egress + # deny. Chart render will FAIL if any agent has gateway.feishu.appId + # set without gateway.feishu.connectionMode="webhook". Switch Feishu + # to webhook mode (or don't use proxy egress). + # # egress.allowDns above still applies. allowHttps/allowHttp/allowIpv6 are # IGNORED when mode=proxy. # - # Rendering fails if proxy.podSelector is empty or proxy.ports is empty - # (would otherwise authorize every pod on every port — fail-open). + # Rendering fails if any of these hold: + # - egress.mode is not exactly "direct" or "proxy" + # - egress.proxy.podSelector is empty + # - egress.proxy.ports is empty + # - any egress.proxy.ports entry omits `port` (would authorize ALL ports) + # - Feishu WebSocket mode is configured on any agent (see above) proxy: # Namespace hosting the proxy pods. Empty = same namespace as the release. namespace: "" From dfc1dae552968d2c604bcaa6a336ef55dca0c03a Mon Sep 17 00:00:00 2001 From: yen <5915590+antigenius0910@users.noreply.github.com> Date: Wed, 15 Jul 2026 00:30:48 -0500 Subject: [PATCH 6/8] refactor(chart): withdraw proxy-mode egress; ship direct-mode + system-proxy only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-5 review revealed that proxy-mode egress cannot be honored end-to-end without a proxy-aware networking refactor across openab-core / openab- gateway. Every prior fix round (rounds 1-4) uncovered another transport that bypasses reqwest and opens WSS directly: - Discord (default) — Serenity's own WSS client via client.start() - Slack Socket Mode — crates/openab-core/src/slack.rs:761 uses tokio_tungstenite::connect_async - External gateway (gateway.deploy=false) — crates/openab-core/src/gateway.rs:818 same pattern - Feishu WebSocket — already guarded in round 4, but the guard only covered the structured chart config path; configToml / envFrom / gateway.env bypasses remained - AgentCore — its own TLS/WebSocket transport The project already documents this in docs/openshell.md: "Unless OAB's networking layer is refactored to be fully HTTP/HTTPS proxy-aware (tunneling WSS through OpenSHELL's L7 proxy), the integration cannot function." Continuing to layer chart-level guards would leave proxy mode practically unusable (Discord is the default transport for most deployments) while giving users a false sense of enterprise egress isolation. Per Option C of the reviewer's third suggestion (also confirmed with the PR author), withdraw proxy mode from this PR entirely. Follow-up RFC will cover proxy-aware egress properly once the Rust networking refactor is scoped. ## What's removed - `networkPolicy.egress.mode` value (was "direct" | "proxy") - `networkPolicy.egress.proxy` config block (namespace / podSelector / ports) - Proxy-mode branch in templates/networkpolicy.yaml egress helper - Feishu WebSocket incompatibility guard (no longer needed) - All proxy-mode validation (empty mode, invalid mode, empty proxy fields, protocol-only ports) - All proxy-mode positive + negative tests - "Proxy-mode CONTRACT" doc block in values.yaml - Proxy-mode language in chart README ## What's kept (all still generally useful) - Opt-in `networkPolicy.enabled` master switch (unchanged) - Direct-mode egress: DNS to kube-system + IPv4/IPv6 HTTPS with configurable metadata exclusions - Agent-to-gateway TCP 8080 egress rule when chart-managed gateway is deployed - `gateway.deploy=false` handling (matches templates/gateway.yaml) - DNS scoping to kube-system (configurable via egress.dnsNamespace) - `metadataExclusions.{ipv4,ipv6}` configurable per cloud - IPv6 support (`allowIpv6`) with AWS IMDS exclusion + additive-semantics doc - CNI-enforcement prerequisite documentation - Defense-in-depth caveat on metadata exclusions - `k8s/networkpolicy.yaml` raw manifest - `gateway.env` / `gateway.envFrom` — reframed as general-purpose env injection (docs no longer suggest proxy usage) ## F2 fix (round 5) — landed as small win regardless Enabled reqwest `system-proxy` feature for the root package and openab-core (previously only openab-gateway had it). Now operators who set HTTPS_PROXY / HTTP_PROXY / NO_PROXY on any variant's pod get reqwest-based HTTP calls (LLM APIs, Slack Web API, remote config, hooks, STT, media) routed through the proxy — WITHOUT this implying the chart supports enforced proxy mode. WSS transports (Slack Socket Mode, external gateway) remain unaffected pending the networking refactor. Cargo comment in openab-core/Cargo.toml calls out that limitation explicitly. Verified with `cargo check --workspace` (clean rebuild across all three reqwest sites) and existing openab-gateway lib test suite. ## Tests Chart: 52/52 pass (was 62, drop is expected — removed 10 proxy-mode tests). Rust: cargo check --workspace clean; openab-gateway lib tests unaffected. helm lint clean. ## Follow-up RFC to file Proxy-aware egress ("networking refactor") — covers: - Replace or fork Serenity to route WSS through HTTP CONNECT - Replace tokio_tungstenite direct calls with a CONNECT-aware alternative (Slack Socket Mode, external gateway, Feishu WebSocket) - AgentCore proxy configuration - AWS SDK proxy configuration (separate from reqwest) - End-to-end integration test with a local Squid/Envoy Refs #1394 Refs #1400 round-5 review by chaodu-agent --- Cargo.toml | 6 +- charts/openab/README.md | 18 +- charts/openab/templates/networkpolicy.yaml | 62 +---- charts/openab/tests/networkpolicy_test.yaml | 279 +++----------------- charts/openab/values.yaml | 101 ++----- crates/openab-core/Cargo.toml | 7 +- 6 files changed, 77 insertions(+), 396 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4ed8f5be7..9c3ca84cd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,11 @@ tracing-subscriber = { version = "0.3", features = ["json", "env-filter"] } clap = { version = "4", features = ["derive"] } anyhow = "1" axum = { version = "0.8", optional = true } -reqwest = { version = "0.12", default-features = false, features = ["rustls-tls", "json"] } +# `system-proxy` re-enables HTTPS_PROXY / HTTP_PROXY / NO_PROXY env-var auto-detection, +# which is a default reqwest feature disabled by `default-features = false`. Lets operators +# route reqwest-based HTTP calls (LLM APIs, Slack Web API, remote config, hooks, STT, media) +# through an HTTP CONNECT proxy by setting the env vars. +reqwest = { version = "0.12", default-features = false, features = ["rustls-tls", "json", "system-proxy"] } serde = { version = "1", features = ["derive"] } serde_json = "1" async-trait = "0.1" diff --git a/charts/openab/README.md b/charts/openab/README.md index 3b1233951..985291cc2 100644 --- a/charts/openab/README.md +++ b/charts/openab/README.md @@ -14,17 +14,15 @@ This page highlights commonly used values and deployment patterns. For the compl | `fullnameOverride` | Override the full generated release name for chart resources. Useful when deploying multiple instances with predictable names. | `""` | | `serviceAccountName` | Chart-global ServiceAccount name attached to every agent pod that doesn't define its own. Empty = cluster `default` SA. Per-agent `agents..serviceAccountName` fully overrides this. Chart references an existing SA only — does not create one. Required for workload identity and pod-level RBAC. | `""` | | `imagePullSecrets` | Chart-global image pull secrets attached to every agent pod that doesn't define its own. Per-agent `agents..imagePullSecrets` fully overrides this. | `[]` | -| `networkPolicy.enabled` | Master switch for the chart-managed NetworkPolicy. Off by default — no `NetworkPolicy` resource is rendered until this flips to `true`, keeping existing releases unaffected. When on: one policy per agent (deny-all ingress + configured egress + TCP 8080 to the chart-managed gateway pod when `gateway.enabled=true` and `gateway.deploy` is not `false`), plus one policy per deployed gateway (port 8080 ingress + same egress). ⚠️ **Prerequisite: your cluster's CNI must enforce NetworkPolicy** (Calico, Cilium, Antrea, EKS with NetworkPolicy add-on, GKE with Dataplane V2 / Calico, AKS with Azure NPM / Calico, etc.). Stock KIND, Docker Desktop, or bare-EKS clusters without a NetworkPolicy-capable CNI silently ignore the resource — verify enforcement with a scratch namespace + deny-all test before relying on this. | `false` | +| `networkPolicy.enabled` | Master switch for the chart-managed NetworkPolicy. Off by default — no `NetworkPolicy` resource is rendered until this flips to `true`, keeping existing releases unaffected. When on: one policy per agent (deny-all ingress + DNS/HTTPS egress + TCP 8080 to the chart-managed gateway pod when `gateway.enabled=true` and `gateway.deploy` is not `false`), plus one policy per deployed gateway (port 8080 ingress + same egress). ⚠️ **Prerequisite: your cluster's CNI must enforce NetworkPolicy** (Calico, Cilium, Antrea, EKS with NetworkPolicy add-on, GKE with Dataplane V2 / Calico, AKS with Azure NPM / Calico, etc.). Stock KIND, Docker Desktop, or bare-EKS clusters without a NetworkPolicy-capable CNI silently ignore the resource — verify enforcement with a scratch namespace + deny-all test before relying on this. ⚠️ **Direct egress only for now** — proxy-mode egress (routing through an operator-supplied HTTP CONNECT proxy) was explored in early rounds of [RFC #1394](https://github.com/openabdev/openab/issues/1394) but requires a proxy-aware networking refactor across `openab-core` / `openab-gateway` (see `docs/openshell.md`). Follow-up RFC to come. | `false` | | `networkPolicy.ingress.extraRules` | Raw `NetworkPolicyIngressRule` entries appended verbatim to every rendered policy. Use for cluster-specific allow-lists (Prometheus scrape, mesh sidecars, etc.). | `[]` | -| `networkPolicy.egress.mode` | Egress topology. Must be exactly `"direct"` or `"proxy"` — anything else (including an empty string) fails the render (fail-closed). `direct` allows egress to the open internet (DNS + HTTPS by default). `proxy` restricts egress to a designated proxy Service — recommended for enterprise / multi-tenant / compliance-driven deployments (see [RFC #1394](https://github.com/openabdev/openab/issues/1394) and OpenSHELL prior art). **Proxy mode enforces the destination allowlist only; you must also configure your workloads to send traffic through the proxy: set `HTTPS_PROXY` / `HTTP_PROXY` on the agent via `agents..env` + `inherit_env` in `configToml`, AND on the gateway via `agents..gateway.env` (the gateway makes its own outbound HTTPS calls to platform APIs).** ⚠️ **Feishu WebSocket mode is incompatible with proxy egress** — the WSS connection bypasses reqwest and does not honor HTTP CONNECT proxies; the chart will fail render unless you set `gateway.feishu.connectionMode: "webhook"`. Chart does not ship the proxy — BYO (OpenSHELL / Squid / Envoy / mitmproxy). | `"direct"` | -| `networkPolicy.egress.allowDns` | Allow DNS egress on UDP+TCP 53 to pods labelled `k8s-app=kube-dns`. Applies in both `direct` and `proxy` modes. | `true` | +| `networkPolicy.egress.allowDns` | Allow DNS egress on UDP+TCP 53 to pods labelled `k8s-app=kube-dns`. | `true` | | `networkPolicy.egress.dnsNamespace` | Trusted namespace hosting the DNS pods. Default `"kube-system"` matches kubeadm / CoreDNS / kube-dns defaults and prevents a co-tenant labelling their own pod `k8s-app=kube-dns` from becoming an approved port-53 destination. Empty string reverts to the legacy any-namespace selector — use only if your DNS runs outside kube-system. | `"kube-system"` | -| `networkPolicy.egress.allowHttps` | (direct mode only) Allow HTTPS (TCP 443) to `0.0.0.0/0` (and `::/0` when `allowIpv6=true`), except CIDRs listed in `metadataExclusions`. Ignored in proxy mode. | `true` | -| `networkPolicy.egress.allowHttp` | (direct mode only) Allow HTTP (TCP 80) with the same metadata exclusion. Off by default. ⚠️ Enable if any of these fetch over HTTP: `agents..configUrl`, lifecycle hooks that download over HTTP (see `docs/hooks.md`), or STT base URLs on HTTP. Prefer HTTPS wherever possible. Ignored in proxy mode. | `false` | -| `networkPolicy.egress.allowIpv6` | (direct mode only) Add `::/0` to the HTTPS/HTTP rules alongside `0.0.0.0/0`, excluding CIDRs from `metadataExclusions.ipv6`. Turn off on IPv4-only clusters. Ignored in proxy mode. | `true` | -| `networkPolicy.egress.metadataExclusions.ipv4` | CIDRs excluded from the direct-mode IPv4 HTTPS/HTTP allow rule. Defaults to `169.254.169.254/32` (AWS/GCP/Azure IMDS). Add IBM Cloud's `161.26.0.0/16` here if applicable. **NetworkPolicy allow rules combine additively — you cannot subtract via `extraRules`; edit this list directly.** ⚠️ **Defense-in-depth only, not a portable IMDS guarantee**: pods can always reach services on their resident node, and CNI implementations differ in whether `ipBlock` matches pre- or post-NAT addresses. Use cloud-provider hardening (IMDSv2 mandatory + hop-limit 1, GCP metadata concealment, Azure metadata endpoint policy) as the primary control. | `["169.254.169.254/32"]` | -| `networkPolicy.egress.metadataExclusions.ipv6` | CIDRs excluded from the direct-mode IPv6 HTTPS/HTTP allow rule. Defaults to `fd00:ec2::254/128` (AWS IPv6 IMDS). Non-AWS IPv6 metadata addresses differ — override this list per cloud. **Same additive-only semantics as `ipv4` above, and same defense-in-depth caveat.** | `["fd00:ec2::254/128"]` | -| `networkPolicy.egress.proxy` | (proxy mode only) Selector for the egress proxy Service — `{namespace, podSelector, ports}`. Render fails if `podSelector` is empty, `ports` is empty, **or any port entry omits `port`** (Kubernetes treats a missing `port` as "all ports for the protocol" — fail-open). Deploy the proxy separately. | `{namespace: "", podSelector: {}, ports: []}` | +| `networkPolicy.egress.allowHttps` | Allow HTTPS (TCP 443) to `0.0.0.0/0` (and `::/0` when `allowIpv6=true`), except CIDRs listed in `metadataExclusions`. | `true` | +| `networkPolicy.egress.allowHttp` | Allow HTTP (TCP 80) with the same metadata exclusion. Off by default. ⚠️ Enable if any of these fetch over HTTP: `agents..configUrl`, lifecycle hooks that download over HTTP (see `docs/hooks.md`), or STT base URLs on HTTP. Prefer HTTPS wherever possible. | `false` | +| `networkPolicy.egress.allowIpv6` | Add `::/0` to the HTTPS/HTTP rules alongside `0.0.0.0/0`, excluding CIDRs from `metadataExclusions.ipv6`. Turn off on IPv4-only clusters. | `true` | +| `networkPolicy.egress.metadataExclusions.ipv4` | CIDRs excluded from the IPv4 HTTPS/HTTP allow rule. Defaults to `169.254.169.254/32` (AWS/GCP/Azure IMDS). Add IBM Cloud's `161.26.0.0/16` here if applicable. **NetworkPolicy allow rules combine additively — you cannot subtract via `extraRules`; edit this list directly.** ⚠️ **Defense-in-depth only, not a portable IMDS guarantee**: pods can always reach services on their resident node, and CNI implementations differ in whether `ipBlock` matches pre- or post-NAT addresses. Use cloud-provider hardening (IMDSv2 mandatory + hop-limit 1, GCP metadata concealment, Azure metadata endpoint policy) as the primary control. | `["169.254.169.254/32"]` | +| `networkPolicy.egress.metadataExclusions.ipv6` | CIDRs excluded from the IPv6 HTTPS/HTTP allow rule. Defaults to `fd00:ec2::254/128` (AWS IPv6 IMDS). Non-AWS IPv6 metadata addresses differ — override this list per cloud. **Same additive-only semantics as `ipv4` above, and same defense-in-depth caveat.** | `["fd00:ec2::254/128"]` | | `networkPolicy.egress.extraRules` | Raw `NetworkPolicyEgressRule` entries appended verbatim. Use to open extra ports or add cluster-specific carve-outs. **Cannot subtract from existing allows — for tighter metadata exclusions use `metadataExclusions` above.** | `[]` | ### Agent values @@ -60,7 +58,7 @@ Each agent lives under `agents.`. | `stt.baseUrl` | STT API base URL. | `"https://api.groq.com/openai/v1"` | | `gateway.enabled` | Enable the gateway config block for webhook-based platforms. | `false` | | `gateway.deploy` | Deploy the gateway Deployment and Service. | `true` | -| `gateway.env` | Additional environment variables for the gateway container (map, `{name: value}`). Common use: `HTTPS_PROXY` / `HTTP_PROXY` / `NO_PROXY` when running under `networkPolicy.egress.mode=proxy` so the gateway can reach platform APIs (Telegram, Teams, etc.) through the same proxy as the agent. | `{}` | +| `gateway.env` | Additional environment variables for the gateway container (map, `{name: value}`). Same shape as agent-level `env`. Useful for feature flags, `RUST_LOG` overrides, or HTTP proxy env vars if the gateway runs behind a transparent proxy. | `{}` | | `gateway.envFrom` | Load env vars for the gateway from existing Secrets or ConfigMaps (list of `secretRef`/`configMapRef` entries). Same shape as agent-level `envFrom`. | `[]` | | `cron.usercronEnabled` | Enable user-provided cron configuration. | `false` | | `cronjobs` | Config-driven scheduled messages for an agent. | `[]` | diff --git a/charts/openab/templates/networkpolicy.yaml b/charts/openab/templates/networkpolicy.yaml index 615665dc5..f89af9737 100644 --- a/charts/openab/templates/networkpolicy.yaml +++ b/charts/openab/templates/networkpolicy.yaml @@ -1,47 +1,8 @@ {{- if .Values.networkPolicy.enabled }} {{- $np := .Values.networkPolicy }} -{{/* ---------- mode and proxy-required-fields validation ---------- */}} -{{/* F2: validate the raw value — do NOT default here. values.yaml already - supplies a default; treating an explicitly empty string as `direct` - silently bypasses the fail-closed contract. */}} -{{- $mode := $np.egress.mode }} -{{- if not (or (eq $mode "direct") (eq $mode "proxy")) }} -{{- fail (printf "networkPolicy.egress.mode must be exactly \"direct\" or \"proxy\" (got %q)" $mode) }} -{{- end }} -{{- if eq $mode "proxy" }} -{{- if not (or $np.egress.proxy.podSelector.matchLabels $np.egress.proxy.podSelector.matchExpressions) }} -{{- fail "networkPolicy.egress.mode=proxy requires networkPolicy.egress.proxy.podSelector.matchLabels or matchExpressions to be non-empty (an empty selector authorizes every pod in the namespace)" }} -{{- end }} -{{- if not $np.egress.proxy.ports }} -{{- fail "networkPolicy.egress.mode=proxy requires networkPolicy.egress.proxy.ports to be non-empty (an empty ports list authorizes every port)" }} -{{- end }} -{{/* F3: every proxy port entry must specify `port`. In a NetworkPolicyPort, - omitting `port` matches all ports for the given protocol, silently - broadening egress to the proxy pods beyond the intended target. */}} -{{- range $i, $p := $np.egress.proxy.ports }} -{{- if not $p.port }} -{{- fail (printf "networkPolicy.egress.mode=proxy requires every networkPolicy.egress.proxy.ports entry to specify port (entry index %d missing port — omitted port matches ALL ports for the given protocol)" $i) }} -{{- end }} -{{- end }} -{{/* F1: Feishu WebSocket mode is incompatible with proxy-only egress — - tokio_tungstenite::connect_async opens WSS directly and does not - consume HTTP CONNECT proxy configuration. Fail render early with a - clear pointer to the webhook fallback. */}} -{{- range $name, $cfg := .Values.agents }} -{{- if ne (include "openab.agentEnabled" $cfg) "false" }} -{{- if and (($cfg.gateway).enabled) (($cfg.gateway).feishu).appId }} -{{- $feishuMode := (($cfg.gateway).feishu).connectionMode | default "websocket" }} -{{- if eq $feishuMode "websocket" }} -{{- fail (printf "networkPolicy.egress.mode=proxy is incompatible with Feishu WebSocket mode for agent %q: the WSS event connection opens TCP 443 directly via tokio_tungstenite and does not honor HTTP CONNECT proxies. Set agents.%s.gateway.feishu.connectionMode=\"webhook\", or disable networkPolicy for this release." $name $name) }} -{{- end }} -{{- end }} -{{- end }} -{{- end }} -{{- end }} {{- range $name, $cfg := .Values.agents }} {{- if ne (include "openab.agentEnabled" $cfg) "false" }} {{- $d := dict "ctx" $ "agent" $name "cfg" $cfg }} -{{/* F2: chart-managed gateway is deployed iff gateway.enabled AND gateway.deploy != false (matches templates/gateway.yaml) */}} {{- $gwDeployed := and (($cfg.gateway).enabled) (ne (($cfg.gateway).deploy | toString) "false") }} {{- $gwD := dict "ctx" $ "agent" (printf "%s-gateway" $name) "cfg" (omit $cfg "nameOverride") }} --- @@ -76,7 +37,6 @@ spec: port: 8080 {{- end }} {{- if $gwDeployed }} -{{/* F3: use the same deploy check as templates/gateway.yaml — avoid orphan NPs */}} --- apiVersion: networking.k8s.io/v1 kind: NetworkPolicy @@ -110,7 +70,7 @@ spec: {{- define "openab.networkPolicyEgress" -}} {{- $np := .np -}} {{- if $np.egress.allowDns -}} -{{/* F4: default DNS destination scoped to a trusted namespace (usually kube-system). +{{/* Default DNS destination scoped to a trusted namespace (usually kube-system). Empty dnsNamespace preserves the legacy any-namespace behaviour for exotic DNS setups. */}} - to: {{- if $np.egress.dnsNamespace }} @@ -132,22 +92,6 @@ spec: - protocol: TCP port: 53 {{- end }} -{{- if eq ($np.egress.mode | default "direct") "proxy" }} -- to: -{{- if $np.egress.proxy.namespace }} -{{/* F7: quote the namespace value — bare all-numeric namespace names YAML-parse as int */}} - - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: {{ $np.egress.proxy.namespace | quote }} - podSelector: - {{- toYaml $np.egress.proxy.podSelector | nindent 8 }} -{{- else }} - - podSelector: - {{- toYaml $np.egress.proxy.podSelector | nindent 8 }} -{{- end }} - ports: - {{- toYaml $np.egress.proxy.ports | nindent 4 }} -{{- else }} {{- $mdxV4 := $np.egress.metadataExclusions.ipv4 | default (list "169.254.169.254/32") }} {{- $mdxV6 := $np.egress.metadataExclusions.ipv6 | default (list "fd00:ec2::254/128") }} {{- if $np.egress.allowHttps }} @@ -157,9 +101,6 @@ spec: except: {{- toYaml $mdxV4 | nindent 10 }} {{- if $np.egress.allowIpv6 }} -{{/* F8/F2: IPv6 exclusions configurable via metadataExclusions.ipv6 — - additive NetworkPolicy semantics mean extraRules cannot subtract - from an existing allow. */}} - ipBlock: cidr: ::/0 except: @@ -185,7 +126,6 @@ spec: - protocol: TCP port: 80 {{- end }} -{{- end }} {{- with $np.egress.extraRules }} {{ toYaml . }} {{- end }} diff --git a/charts/openab/tests/networkpolicy_test.yaml b/charts/openab/tests/networkpolicy_test.yaml index 0d34510eb..cc2d9935d 100644 --- a/charts/openab/tests/networkpolicy_test.yaml +++ b/charts/openab/tests/networkpolicy_test.yaml @@ -1,4 +1,4 @@ -suite: NetworkPolicy support (opt-in, direct + proxy egress modes) +suite: NetworkPolicy support (opt-in, direct egress only) templates: - templates/networkpolicy.yaml - templates/gateway.yaml @@ -10,7 +10,7 @@ tests: - hasDocuments: count: 0 - - it: renders a single agent NetworkPolicy in direct mode (gateway off) with deny-all ingress, DNS scoped to kube-system, and IPv4+IPv6 HTTPS egress + - it: renders a single agent NetworkPolicy (gateway off) with deny-all ingress, DNS scoped to kube-system, and IPv4+IPv6 HTTPS egress template: templates/networkpolicy.yaml set: networkPolicy.enabled: true @@ -63,7 +63,7 @@ tests: - protocol: TCP port: 443 - - it: renders both agent and gateway NetworkPolicies in direct mode when gateway is deployed, and the agent policy authorizes TCP 8080 to the gateway pod (F2) + - it: renders both agent and gateway NetworkPolicies when gateway is deployed, and the agent policy authorizes TCP 8080 to the gateway pod template: templates/networkpolicy.yaml set: networkPolicy.enabled: true @@ -104,7 +104,7 @@ tests: port: 8080 documentIndex: 0 - - it: does NOT render a gateway NetworkPolicy or an agent→gateway egress rule when gateway.enabled=true but gateway.deploy=false (F3 regression — matches templates/gateway.yaml condition) + - it: does NOT render a gateway NetworkPolicy or an agent→gateway egress rule when gateway.enabled=true but gateway.deploy=false (matches templates/gateway.yaml condition) template: templates/networkpolicy.yaml set: networkPolicy.enabled: true @@ -130,7 +130,7 @@ tests: port: 8080 any: true - - it: renders HTTP egress rule (IPv4+IPv6) when egress.allowHttp is true (direct mode) + - it: renders HTTP egress rule (IPv4+IPv6) when egress.allowHttp is true template: templates/networkpolicy.yaml set: networkPolicy.enabled: true @@ -152,7 +152,7 @@ tests: - protocol: TCP port: 80 - - it: omits IPv6 ipBlock when allowIpv6=false (direct mode, IPv4-only cluster) (F8) + - it: omits IPv6 ipBlock when allowIpv6=false (IPv4-only cluster) template: templates/networkpolicy.yaml set: networkPolicy.enabled: true @@ -179,7 +179,7 @@ tests: - fd00:ec2::254/128 any: true - - it: falls back to any-namespace DNS when egress.dnsNamespace is empty string (F4 legacy escape hatch) + - it: falls back to any-namespace DNS when egress.dnsNamespace is empty string (legacy escape hatch) template: templates/networkpolicy.yaml set: networkPolicy.enabled: true @@ -199,105 +199,6 @@ tests: - protocol: TCP port: 53 - - it: restricts agent egress to the designated proxy Service in proxy mode (F5 — enforcement only) - template: templates/networkpolicy.yaml - set: - networkPolicy.enabled: true - networkPolicy.egress.mode: proxy - networkPolicy.egress.proxy.namespace: egress-proxy - networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell - networkPolicy.egress.proxy.ports: - - protocol: TCP - port: 3128 - asserts: - - contains: - path: spec.egress - content: - to: - - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: egress-proxy - podSelector: - matchLabels: - app: openshell - ports: - - protocol: TCP - port: 3128 - - notContains: - path: spec.egress - content: - to: - - ipBlock: - cidr: 0.0.0.0/0 - except: - - 169.254.169.254/32 - - ipBlock: - cidr: ::/0 - except: - - fd00:ec2::254/128 - ports: - - protocol: TCP - port: 443 - any: true - - - it: quotes an all-numeric proxy namespace so it renders as a string, not a YAML integer (F7) - template: templates/networkpolicy.yaml - set: - networkPolicy.enabled: true - networkPolicy.egress.mode: proxy - networkPolicy.egress.proxy.namespace: "1234" - networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell - networkPolicy.egress.proxy.ports: - - protocol: TCP - port: 3128 - asserts: - - equal: - path: spec.egress[1].to[0].namespaceSelector.matchLabels["kubernetes.io/metadata.name"] - value: "1234" - - - it: applies proxy-mode egress to both agent and gateway policies when gateway is deployed - template: templates/networkpolicy.yaml - set: - networkPolicy.enabled: true - networkPolicy.egress.mode: proxy - networkPolicy.egress.proxy.namespace: egress-proxy - networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell - networkPolicy.egress.proxy.ports: - - protocol: TCP - port: 3128 - agents.kiro.gateway.enabled: true - asserts: - - hasDocuments: - count: 2 - - contains: - path: spec.egress - content: - to: - - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: egress-proxy - podSelector: - matchLabels: - app: openshell - ports: - - protocol: TCP - port: 3128 - documentIndex: 0 - - contains: - path: spec.egress - content: - to: - - namespaceSelector: - matchLabels: - kubernetes.io/metadata.name: egress-proxy - podSelector: - matchLabels: - app: openshell - ports: - - protocol: TCP - port: 3128 - documentIndex: 1 - - it: appends ingress.extraRules verbatim to the agent policy template: templates/networkpolicy.yaml set: @@ -322,7 +223,7 @@ tests: - protocol: TCP port: 9090 - - it: locks egress down to only extraRules when allowDns and allowHttps are false (direct mode) + - it: locks egress down to only extraRules when allowDns and allowHttps are false template: templates/networkpolicy.yaml set: networkPolicy.enabled: true @@ -370,163 +271,61 @@ tests: - protocol: TCP port: 443 - # ── F1 negative tests: fail-closed validation ───────────────────────────── - - - it: fails rendering when egress.mode is not exactly "direct" or "proxy" (F1) + - it: uses configurable metadataExclusions for both IPv4 and IPv6 in the HTTPS rule template: templates/networkpolicy.yaml set: networkPolicy.enabled: true - networkPolicy.egress.mode: proxxy - asserts: - - failedTemplate: - errorMessage: 'networkPolicy.egress.mode must be exactly "direct" or "proxy" (got "proxxy")' - - - it: fails rendering when mode=proxy but proxy.podSelector is empty (F1) - template: templates/networkpolicy.yaml - set: - networkPolicy.enabled: true - networkPolicy.egress.mode: proxy - networkPolicy.egress.proxy.ports: - - protocol: TCP - port: 3128 - asserts: - - failedTemplate: - errorMessage: 'networkPolicy.egress.mode=proxy requires networkPolicy.egress.proxy.podSelector.matchLabels or matchExpressions to be non-empty (an empty selector authorizes every pod in the namespace)' - - - it: fails rendering when mode=proxy but proxy.ports is empty (F1) - template: templates/networkpolicy.yaml - set: - networkPolicy.enabled: true - networkPolicy.egress.mode: proxy - networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell + networkPolicy.egress.metadataExclusions.ipv4: + - 169.254.169.254/32 + - 169.254.170.2/32 + networkPolicy.egress.metadataExclusions.ipv6: + - fd00:beef::254/128 asserts: - - failedTemplate: - errorMessage: 'networkPolicy.egress.mode=proxy requires networkPolicy.egress.proxy.ports to be non-empty (an empty ports list authorizes every port)' + - contains: + path: spec.egress + content: + to: + - ipBlock: + cidr: 0.0.0.0/0 + except: + - 169.254.169.254/32 + - 169.254.170.2/32 + - ipBlock: + cidr: ::/0 + except: + - fd00:beef::254/128 + ports: + - protocol: TCP + port: 443 - # ── Round-2 review fixes ───────────────────────────────────────────────── + # ── Gateway env / envFrom (general-purpose env injection) ───────────────── - - it: injects gateway.env into the gateway Deployment so proxy-mode gateway can route outbound HTTPS through the proxy (round-2 F1) + - it: renders gateway.env into the gateway Deployment template: templates/gateway.yaml set: agents.kiro.gateway.enabled: true agents.kiro.gateway.env: - HTTPS_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" - HTTP_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" - NO_PROXY: ".svc,.cluster.local,10.0.0.0/8" + MY_FLAG: "on" + RUST_LOG: "openab_gateway=debug" asserts: - contains: path: spec.template.spec.containers[0].env content: - name: HTTPS_PROXY - value: "http://openshell.egress-proxy.svc.cluster.local:3128" - documentIndex: 0 - - contains: - path: spec.template.spec.containers[0].env - content: - name: HTTP_PROXY - value: "http://openshell.egress-proxy.svc.cluster.local:3128" - documentIndex: 0 - - contains: - path: spec.template.spec.containers[0].env - content: - name: NO_PROXY - value: ".svc,.cluster.local,10.0.0.0/8" + name: MY_FLAG + value: "on" documentIndex: 0 - - it: renders gateway.envFrom on the gateway Deployment (round-2 F1) + - it: renders gateway.envFrom on the gateway Deployment template: templates/gateway.yaml set: agents.kiro.gateway.enabled: true agents.kiro.gateway.envFrom: - secretRef: - name: proxy-credentials + name: gateway-extras asserts: - equal: path: spec.template.spec.containers[0].envFrom value: - secretRef: - name: proxy-credentials + name: gateway-extras documentIndex: 0 - - # ── Round-4 review fixes: fail-closed validation gaps ────────────────── - - - it: fails rendering when egress.mode is an explicitly empty string (round-4 F2) - template: templates/networkpolicy.yaml - set: - networkPolicy.enabled: true - networkPolicy.egress.mode: "" - asserts: - - failedTemplate: - errorMessage: 'networkPolicy.egress.mode must be exactly "direct" or "proxy" (got "")' - - - it: fails rendering when any proxy.ports entry omits `port` (round-4 F3) - template: templates/networkpolicy.yaml - set: - networkPolicy.enabled: true - networkPolicy.egress.mode: proxy - networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell - networkPolicy.egress.proxy.ports: - - protocol: TCP - asserts: - - failedTemplate: - errorMessage: 'networkPolicy.egress.mode=proxy requires every networkPolicy.egress.proxy.ports entry to specify port (entry index 0 missing port — omitted port matches ALL ports for the given protocol)' - - - it: fails rendering when proxy mode is combined with Feishu WebSocket mode (round-4 F1) - template: templates/networkpolicy.yaml - set: - networkPolicy.enabled: true - networkPolicy.egress.mode: proxy - networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell - networkPolicy.egress.proxy.ports: - - protocol: TCP - port: 3128 - agents.kiro.gateway.enabled: true - agents.kiro.gateway.feishu.appId: cli_test - agents.kiro.gateway.feishu.appSecret: secret - asserts: - - failedTemplate: - errorMessage: 'networkPolicy.egress.mode=proxy is incompatible with Feishu WebSocket mode for agent "kiro": the WSS event connection opens TCP 443 directly via tokio_tungstenite and does not honor HTTP CONNECT proxies. Set agents.kiro.gateway.feishu.connectionMode="webhook", or disable networkPolicy for this release.' - - - it: renders successfully when proxy mode is combined with Feishu WEBHOOK mode (round-4 F1 positive case) - template: templates/networkpolicy.yaml - set: - networkPolicy.enabled: true - networkPolicy.egress.mode: proxy - networkPolicy.egress.proxy.podSelector.matchLabels.app: openshell - networkPolicy.egress.proxy.ports: - - protocol: TCP - port: 3128 - agents.kiro.gateway.enabled: true - agents.kiro.gateway.feishu.appId: cli_test - agents.kiro.gateway.feishu.appSecret: secret - agents.kiro.gateway.feishu.connectionMode: webhook - asserts: - - hasDocuments: - count: 2 - - - it: uses configurable metadataExclusions for both IPv4 and IPv6 in direct-mode HTTPS rule (round-2 F2) - template: templates/networkpolicy.yaml - set: - networkPolicy.enabled: true - networkPolicy.egress.metadataExclusions.ipv4: - - 169.254.169.254/32 - - 169.254.170.2/32 - networkPolicy.egress.metadataExclusions.ipv6: - - fd00:beef::254/128 - asserts: - - contains: - path: spec.egress - content: - to: - - ipBlock: - cidr: 0.0.0.0/0 - except: - - 169.254.169.254/32 - - 169.254.170.2/32 - - ipBlock: - cidr: ::/0 - except: - - fd00:beef::254/128 - ports: - - protocol: TCP - port: 443 diff --git a/charts/openab/values.yaml b/charts/openab/values.yaml index 59b408269..fff5a2f03 100644 --- a/charts/openab/values.yaml +++ b/charts/openab/values.yaml @@ -67,17 +67,18 @@ networkPolicy: extraRules: [] egress: - # Egress topology. Must be exactly "direct" or "proxy" — invalid values - # fail the render (fail-closed). - # - direct: allow DNS + HTTPS (and optionally HTTP) to the open internet - # with cloud metadata (169.254.169.254 / fd00:ec2::254) excluded. - # Hobbyist / dev-cluster / small-team path. - # - proxy: allow DNS + egress restricted to a designated proxy Service. - # Enterprise path per OpenSHELL prior art. See "proxy contract" note below. - mode: direct + # ⚠️ Egress topology is DIRECT only for now. + # Proxy-mode egress (routing all traffic through an operator-supplied + # HTTP CONNECT proxy — the OpenSHELL / Squid / Envoy pattern) was + # explored in early rounds of RFC #1394 but requires a proxy-aware + # networking refactor across openab-core / openab-gateway (see + # docs/openshell.md). Discord, Slack Socket Mode, external gateway, + # and Feishu default connections all bypass reqwest and open WSS + # directly — they cannot be authorized through HTTP CONNECT today. + # The chart therefore only ships direct egress. A follow-up RFC will + # cover proxy egress once the Rust networking refactor lands. # Allow DNS egress on UDP/TCP 53 to pods labelled k8s-app=kube-dns. - # Applies in both direct and proxy modes. allowDns: true # Trusted namespace hosting the DNS pods. Default "kube-system" matches # kubeadm/CoreDNS/kube-dns defaults and prevents a co-tenant from labelling @@ -86,21 +87,20 @@ networkPolicy: # your DNS runs outside kube-system, or set to your DNS namespace directly. dnsNamespace: "kube-system" - # ── mode: direct ──────────────────────────────────────────────────────── # Allow HTTPS (TCP 443). Excludes cloud metadata (see metadataExclusions # below) — blocks a common IAM-credential-theft path from a compromised - # agent pod. Ignored when mode=proxy. + # agent pod. allowHttps: true # Allow HTTP (TCP 80) with the same metadata exclusion. Off by default. # ⚠️ Enable if any of these fetch over HTTP: `agents..configUrl`, # lifecycle hooks that download over HTTP (see docs/hooks.md), or STT - # base URLs on HTTP. Prefer HTTPS wherever possible. Ignored when mode=proxy. + # base URLs on HTTP. Prefer HTTPS wherever possible. allowHttp: false # Add ::/0 to HTTPS/HTTP rules alongside 0.0.0.0/0 for IPv6 egress. # Turn off on IPv4-only clusters if you prefer explicit denial to - # allowing unused v6 ranges. Ignored when mode=proxy. + # allowing unused v6 ranges. allowIpv6: true - # Cloud metadata CIDRs excluded from the direct-mode HTTPS/HTTP rules. + # Cloud metadata CIDRs excluded from the HTTPS/HTTP rules. # NetworkPolicy allow rules combine additively — you CANNOT add another # exclusion via extraRules; you must edit these lists directly (or set # allowHttps=false + supply a complete replacement rule via extraRules). @@ -128,66 +128,6 @@ networkPolicy: ipv6: - fd00:ec2::254/128 - # ── mode: proxy ───────────────────────────────────────────────────────── - # ⚠️ Proxy-mode CONTRACT - # NetworkPolicy only ENFORCES the destination allowlist. It does NOT - # redirect traffic through the proxy. You must configure BOTH the agent - # AND the chart-managed gateway (if deployed) to route their outbound - # traffic through the proxy — otherwise the NetworkPolicy deny will - # break platform API calls (Telegram replies, Teams OAuth, LLM APIs). - # - # 1. Agent — set proxy env vars via agents..env AND add them to - # inherit_env in configToml so ACP children (claude-agent-acp, - # kiro-cli, opencode, etc.) inherit them: - # - # agents..env: - # HTTPS_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" - # HTTP_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" - # NO_PROXY: ".svc,.cluster.local,10.0.0.0/8" - # - # agents..configToml: | - # [agent] - # inherit_env = ["HTTPS_PROXY","HTTP_PROXY","NO_PROXY"] - # - # 2. Gateway — the gateway pod ALSO makes outbound HTTPS calls (Telegram - # replies, Teams OAuth token exchange, Feishu/WeCom APIs, etc.). Set - # the same env vars on it via the new agents..gateway.env field: - # - # agents..gateway.env: - # HTTPS_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" - # HTTP_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" - # NO_PROXY: ".svc,.cluster.local,10.0.0.0/8" - # - # Chart does NOT ship the proxy — deploy it separately (OpenSHELL, - # Squid, Envoy, mitmproxy, or anything speaking HTTP CONNECT). - # - # ⚠️ Feishu WebSocket mode is INCOMPATIBLE with proxy egress. - # The Feishu adapter opens its event stream via - # tokio_tungstenite::connect_async, which does NOT honor HTTP CONNECT - # proxy env vars — even with reqwest's system-proxy feature enabled on - # the gateway. In proxy mode, the WSS connection tries direct TCP 443 - # to the Feishu endpoint and is blocked by the NetworkPolicy egress - # deny. Chart render will FAIL if any agent has gateway.feishu.appId - # set without gateway.feishu.connectionMode="webhook". Switch Feishu - # to webhook mode (or don't use proxy egress). - # - # egress.allowDns above still applies. allowHttps/allowHttp/allowIpv6 are - # IGNORED when mode=proxy. - # - # Rendering fails if any of these hold: - # - egress.mode is not exactly "direct" or "proxy" - # - egress.proxy.podSelector is empty - # - egress.proxy.ports is empty - # - any egress.proxy.ports entry omits `port` (would authorize ALL ports) - # - Feishu WebSocket mode is configured on any agent (see above) - proxy: - # Namespace hosting the proxy pods. Empty = same namespace as the release. - namespace: "" - # Kubernetes label selector for the proxy pods. - podSelector: {} - # Ports on the proxy pods that agents should reach. - ports: [] - # extraRules: raw NetworkPolicyEgressRule entries appended verbatim. # Use to open extra ports or add cluster-specific carve-outs. extraRules: [] @@ -556,15 +496,10 @@ agents: strategy: "Recreate" # Recreate (default, prevents concurrent WS conflicts) or RollingUpdate resources: {} # e.g. { requests: { cpu: 50m, memory: 64Mi }, limits: { memory: 128Mi } } rustLog: "info" # RUST_LOG for gateway container (e.g. "openab_gateway=debug") - # Additional environment variables for the gateway container. - # Useful for injecting HTTPS_PROXY / HTTP_PROXY / NO_PROXY when running - # under networkPolicy.egress.mode=proxy (the gateway also makes outbound - # HTTPS calls to platform APIs — Telegram replies, Teams OAuth, etc.). - # Example: - # env: - # HTTPS_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" - # HTTP_PROXY: "http://openshell.egress-proxy.svc.cluster.local:3128" - # NO_PROXY: ".svc,.cluster.local,10.0.0.0/8" + # Additional environment variables for the gateway container (map, + # {name: value}). Same shape as agent-level `env`. Useful for feature + # flags, RUST_LOG overrides, HTTP proxy env vars if you're running the + # gateway behind a transparent proxy, etc. env: {} # Load additional env vars into the gateway from existing Secrets or ConfigMaps. # Same shape as agent-level `envFrom` (list of secretRef/configMapRef entries). diff --git a/crates/openab-core/Cargo.toml b/crates/openab-core/Cargo.toml index d375a7f4b..55fbc09a0 100644 --- a/crates/openab-core/Cargo.toml +++ b/crates/openab-core/Cargo.toml @@ -21,7 +21,12 @@ futures-util = "0.3" rand = "0.8" clap = { version = "4", features = ["derive"] } rpassword = "7" -reqwest = { version = "0.12", default-features = false, features = ["rustls-tls", "multipart", "json", "blocking", "stream"] } +# `system-proxy` re-enables HTTPS_PROXY / HTTP_PROXY / NO_PROXY env-var auto-detection +# (default reqwest feature disabled by `default-features = false`). Note: WebSocket +# transports in this crate (Slack Socket Mode, external gateway WSS) use +# tokio_tungstenite directly and do NOT honor these env vars — proxy-aware WSS +# tunneling requires the openab networking refactor tracked in docs/openshell.md. +reqwest = { version = "0.12", default-features = false, features = ["rustls-tls", "multipart", "json", "blocking", "stream", "system-proxy"] } bytes = "1" base64 = "0.22" image = { version = "0.25", default-features = false, features = ["jpeg", "png", "gif", "webp"] } From 802cdd95065402750cb9b99a4a3ae2f91047f8f7 Mon Sep 17 00:00:00 2001 From: yen <5915590+antigenius0910@users.noreply.github.com> Date: Wed, 15 Jul 2026 01:38:27 -0500 Subject: [PATCH 7/8] =?UTF-8?q?fix(chart,agent):=20address=20PR=20#1400=20?= =?UTF-8?q?round-6=20review=20=E2=80=94=20native=20agent=20proxy,=20extern?= =?UTF-8?q?al-gateway=20docs,=20stale=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses all three round-6 findings from chaodu-agent on commit dfc1dae. F1 (custom-port destinations require extraRules): allowHttps opens only TCP 443 and allowHttp only TCP 80. Supported openab configurations that use other ports were silently broken when enabling NetworkPolicy: (a) external config-only gateway (gateway.deploy=false) listens on TCP 8080; (b) self-hosted STT often runs on custom ports (e.g. http://192.168.1.100:8080/v1); (c) internal LLM proxies like LiteLLM commonly bind :4000. Fix (docs-only, since a chart-level default for arbitrary custom ports would be too broad): - Added a ⚠️ warning block above `networkPolicy.egress.extraRules` in values.yaml enumerating the three common cases and providing a concrete extraRules recipe for the external-gateway case. - Chart README row for `extraRules` mirrors the caveat. - Chart README row for `gateway.deploy` calls out that setting it to false removes the chart-managed agent→gateway TCP 8080 exception, so operators must supply an extraRules entry. - New helm-unittest test asserts the documented extraRules recipe for external-gateway TCP 8080 renders when gateway.deploy=false (also verifies that no chart-managed agent→gateway rule leaks in that path). F2 (native openab-agent proxy claim was incomplete): The PR body claimed reqwest system-proxy coverage for LLM API calls, but openab-agent is workspace-excluded (root Cargo.toml `exclude` list) and has its own Cargo.toml + Cargo.lock. Its reqwest 0.12 and reqwest013 (0.13, aliased for rmcp OAuth) declarations both had `default-features = false` without system-proxy. Setting HTTPS_PROXY on the native agent's pod would not route LLM requests (src/llm.rs:293/663/ 1033 construct clients via reqwest::Client::new() without explicit proxy config). Fix: added "system-proxy" to both reqwest and reqwest013 features in openab-agent/Cargo.toml, with an inline comment matching the root/core style. `cargo check` in openab-agent clean; Cargo.lock updated for the new feature dep. F3 (stale withdrawn-mode reference in gateway Cargo comment): The comment I added in round 3 referenced networkPolicy.egress.mode=proxy, which was removed in the round-5 withdraw commit (dfc1dae). Rewrote the comment to match root/core framing: general operator-supplied proxy env vars, with an explicit note that WSS transports (Feishu WSS via tokio_tungstenite) don't honor these env vars — that's the follow-up networking refactor. Tests: 53/53 helm-unittest pass (was 52, +1 external-gateway extraRules render test). helm lint clean. cargo check on openab-agent workspace clean. Refs #1394 Refs #1400 round-6 review by chaodu-agent --- charts/openab/README.md | 4 +- charts/openab/tests/networkpolicy_test.yaml | 45 +++++++++++++++++++ charts/openab/values.yaml | 28 ++++++++++++ crates/openab-gateway/Cargo.toml | 11 +++-- openab-agent/Cargo.lock | 48 ++++++++++++++++++++- openab-agent/Cargo.toml | 11 +++-- 6 files changed, 136 insertions(+), 11 deletions(-) diff --git a/charts/openab/README.md b/charts/openab/README.md index 985291cc2..f74479f2b 100644 --- a/charts/openab/README.md +++ b/charts/openab/README.md @@ -23,7 +23,7 @@ This page highlights commonly used values and deployment patterns. For the compl | `networkPolicy.egress.allowIpv6` | Add `::/0` to the HTTPS/HTTP rules alongside `0.0.0.0/0`, excluding CIDRs from `metadataExclusions.ipv6`. Turn off on IPv4-only clusters. | `true` | | `networkPolicy.egress.metadataExclusions.ipv4` | CIDRs excluded from the IPv4 HTTPS/HTTP allow rule. Defaults to `169.254.169.254/32` (AWS/GCP/Azure IMDS). Add IBM Cloud's `161.26.0.0/16` here if applicable. **NetworkPolicy allow rules combine additively — you cannot subtract via `extraRules`; edit this list directly.** ⚠️ **Defense-in-depth only, not a portable IMDS guarantee**: pods can always reach services on their resident node, and CNI implementations differ in whether `ipBlock` matches pre- or post-NAT addresses. Use cloud-provider hardening (IMDSv2 mandatory + hop-limit 1, GCP metadata concealment, Azure metadata endpoint policy) as the primary control. | `["169.254.169.254/32"]` | | `networkPolicy.egress.metadataExclusions.ipv6` | CIDRs excluded from the IPv6 HTTPS/HTTP allow rule. Defaults to `fd00:ec2::254/128` (AWS IPv6 IMDS). Non-AWS IPv6 metadata addresses differ — override this list per cloud. **Same additive-only semantics as `ipv4` above, and same defense-in-depth caveat.** | `["fd00:ec2::254/128"]` | -| `networkPolicy.egress.extraRules` | Raw `NetworkPolicyEgressRule` entries appended verbatim. Use to open extra ports or add cluster-specific carve-outs. **Cannot subtract from existing allows — for tighter metadata exclusions use `metadataExclusions` above.** | `[]` | +| `networkPolicy.egress.extraRules` | Raw `NetworkPolicyEgressRule` entries appended verbatim. Use to open extra ports or add cluster-specific carve-outs. **Cannot subtract from existing allows — for tighter metadata exclusions use `metadataExclusions` above.** ⚠️ **Non-80/443 destinations require an explicit entry here** — enabling `networkPolicy.enabled=true` silently breaks: (a) external config-only gateway (`gateway.deploy=false` + `gateway.url` on TCP 8080), (b) self-hosted STT on non-standard ports (e.g. `http://192.168.1.100:8080/v1`), (c) internal LLM proxies (LiteLLM on 4000, etc.). See `values.yaml` for a concrete `extraRules` recipe for the external-gateway case. | `[]` | ### Agent values @@ -57,7 +57,7 @@ Each agent lives under `agents.`. | `stt.model` | STT model name. | `"whisper-large-v3-turbo"` | | `stt.baseUrl` | STT API base URL. | `"https://api.groq.com/openai/v1"` | | `gateway.enabled` | Enable the gateway config block for webhook-based platforms. | `false` | -| `gateway.deploy` | Deploy the gateway Deployment and Service. | `true` | +| `gateway.deploy` | Deploy the gateway Deployment and Service. ⚠️ When `false` (config-only / external gateway), the chart-managed agent→gateway TCP 8080 egress rule is NOT rendered — if `networkPolicy.enabled=true` you must add a matching entry to `networkPolicy.egress.extraRules` targeting your external gateway pod/service, or agent WSS to the external gateway will be blocked. See values.yaml for a concrete recipe. | `true` | | `gateway.env` | Additional environment variables for the gateway container (map, `{name: value}`). Same shape as agent-level `env`. Useful for feature flags, `RUST_LOG` overrides, or HTTP proxy env vars if the gateway runs behind a transparent proxy. | `{}` | | `gateway.envFrom` | Load env vars for the gateway from existing Secrets or ConfigMaps (list of `secretRef`/`configMapRef` entries). Same shape as agent-level `envFrom`. | `[]` | | `cron.usercronEnabled` | Enable user-provided cron configuration. | `false` | diff --git a/charts/openab/tests/networkpolicy_test.yaml b/charts/openab/tests/networkpolicy_test.yaml index cc2d9935d..4f9a58cdc 100644 --- a/charts/openab/tests/networkpolicy_test.yaml +++ b/charts/openab/tests/networkpolicy_test.yaml @@ -271,6 +271,51 @@ tests: - protocol: TCP port: 443 + - it: documented extraRules recipe for external-gateway TCP 8080 renders when gateway.deploy=false (round-6 F1) + template: templates/networkpolicy.yaml + set: + networkPolicy.enabled: true + networkPolicy.egress.extraRules: + - to: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: gateway-ns + podSelector: + matchLabels: + app: openab-gateway + ports: + - protocol: TCP + port: 8080 + agents.kiro.gateway.enabled: true + agents.kiro.gateway.deploy: false + asserts: + - hasDocuments: + count: 1 + - contains: + path: spec.egress + content: + to: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: gateway-ns + podSelector: + matchLabels: + app: openab-gateway + ports: + - protocol: TCP + port: 8080 + # No chart-managed agent→gateway rule (gateway.deploy=false) + - notContains: + path: spec.egress + content: + to: + - podSelector: + matchLabels: + app.kubernetes.io/name: openab + app.kubernetes.io/instance: RELEASE-NAME + app.kubernetes.io/component: kiro-gateway + any: true + - it: uses configurable metadataExclusions for both IPv4 and IPv6 in the HTTPS rule template: templates/networkpolicy.yaml set: diff --git a/charts/openab/values.yaml b/charts/openab/values.yaml index fff5a2f03..c6d8669de 100644 --- a/charts/openab/values.yaml +++ b/charts/openab/values.yaml @@ -130,6 +130,34 @@ networkPolicy: # extraRules: raw NetworkPolicyEgressRule entries appended verbatim. # Use to open extra ports or add cluster-specific carve-outs. + # + # ⚠️ Non-80/443 destinations REQUIRE an explicit entry here. + # allowHttps only opens TCP 443, and allowHttp only opens TCP 80. If any + # of your supported openab configurations reach services on other ports + # you MUST add a rule for them, otherwise enabling NetworkPolicy silently + # breaks that traffic. Common cases: + # + # 1. External (config-only) gateway — agents..gateway.deploy=false + # with agents..gateway.url pointing at ws://openab-gateway:8080/ws + # does NOT get the chart-managed agent→gateway port-8080 exception + # (that only fires when gateway.deploy is true). Add: + # + # extraRules: + # - to: + # - namespaceSelector: + # matchLabels: + # kubernetes.io/metadata.name: gateway-ns + # podSelector: + # matchLabels: + # app: openab-gateway + # ports: + # - protocol: TCP + # port: 8080 + # + # 2. Self-hosted STT on a non-standard port (e.g. http://192.168.1.100:8080/v1) + # — add an ipBlock + port entry (see docs/stt.md for hostname/CIDR examples). + # + # 3. Internal LLM proxy on a custom port (e.g. LiteLLM on 4000) — same shape. extraRules: [] agents: diff --git a/crates/openab-gateway/Cargo.toml b/crates/openab-gateway/Cargo.toml index f7854095e..8791a4729 100644 --- a/crates/openab-gateway/Cargo.toml +++ b/crates/openab-gateway/Cargo.toml @@ -11,10 +11,13 @@ tokio-tungstenite = { version = "0.21", features = ["rustls-tls-webpki-roots"] } futures-util = "0.3" serde = { version = "1", features = ["derive"] } serde_json = "1" -# `system-proxy` re-enables HTTPS_PROXY / HTTP_PROXY / NO_PROXY env-var auto-detection, -# which is a default reqwest feature disabled by `default-features = false`. Required so -# the gateway respects proxy env vars injected via chart `agents..gateway.env` when -# running under `networkPolicy.egress.mode=proxy`. See PR #1400. +# `system-proxy` re-enables HTTPS_PROXY / HTTP_PROXY / NO_PROXY env-var auto-detection +# (default reqwest feature disabled by `default-features = false`). Lets operators route +# reqwest-based gateway HTTP calls (Telegram/Teams/Feishu/WeCom/GoogleChat/LINE APIs) +# through an HTTP CONNECT proxy by setting the env vars on the pod. +# Note: WebSocket transports (Feishu WSS) use tokio_tungstenite directly and do NOT +# honor these env vars — that requires the openab networking refactor tracked in +# docs/openshell.md. reqwest = { version = "0.12", default-features = false, features = ["rustls-tls", "json", "system-proxy"] } tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } diff --git a/openab-agent/Cargo.lock b/openab-agent/Cargo.lock index 3f23465d2..1639209ae 100644 --- a/openab-agent/Cargo.lock +++ b/openab-agent/Cargo.lock @@ -319,6 +319,16 @@ version = "0.10.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a6ef517f0926dd24a1582492c791b6a4818a4d94e789a334894aa15b0d12f55c" +[[package]] +name = "core-foundation" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91e195e091a93c46f7102ec7818a2aa394e1e1771c3ab4825963fa03e45afb8f" +dependencies = [ + "core-foundation-sys", + "libc", +] + [[package]] name = "core-foundation" version = "0.10.1" @@ -784,9 +794,11 @@ dependencies = [ "percent-encoding", "pin-project-lite", "socket2", + "system-configuration", "tokio", "tower-service", "tracing", + "windows-registry", ] [[package]] @@ -1826,7 +1838,7 @@ version = "0.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "26d1e2536ce4f35f4846aa13bff16bd0ff40157cdb14cc056c7b14ba41233ba0" dependencies = [ - "core-foundation", + "core-foundation 0.10.1", "core-foundation-sys", "jni", "log", @@ -1902,7 +1914,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b7f4bc775c73d9a02cde8bf7b2ec4c9d12743edf609006c7facc23998404cd1d" dependencies = [ "bitflags", - "core-foundation", + "core-foundation 0.10.1", "core-foundation-sys", "libc", "security-framework-sys", @@ -2137,6 +2149,27 @@ dependencies = [ "syn", ] +[[package]] +name = "system-configuration" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a13f3d0daba03132c0aa9767f98351b3488edc2c100cda2d2ec2b04f3d8d3c8b" +dependencies = [ + "bitflags", + "core-foundation 0.9.4", + "system-configuration-sys", +] + +[[package]] +name = "system-configuration-sys" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e1d1b10ced5ca923a1fcb8d03e96b8d3268065d724548c0211415ff6ac6bac4" +dependencies = [ + "core-foundation-sys", + "libc", +] + [[package]] name = "temp-env" version = "0.3.6" @@ -2782,6 +2815,17 @@ dependencies = [ "windows-link", ] +[[package]] +name = "windows-registry" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "02752bf7fbdcce7f2a27a742f798510f3e5ad88dbe84871e5168e2120c3d5720" +dependencies = [ + "windows-link", + "windows-result", + "windows-strings", +] + [[package]] name = "windows-result" version = "0.4.1" diff --git a/openab-agent/Cargo.toml b/openab-agent/Cargo.toml index 5f146ca21..d99bcd0cb 100644 --- a/openab-agent/Cargo.toml +++ b/openab-agent/Cargo.toml @@ -9,11 +9,16 @@ description = "Native Rust coding agent with built-in ACP support" tokio = { version = "1", features = ["full"] } serde = { version = "1", features = ["derive"] } serde_json = "1" -reqwest = { version = "0.12", default-features = false, features = ["rustls-tls", "json"] } +# `system-proxy` re-enables HTTPS_PROXY / HTTP_PROXY / NO_PROXY env-var auto-detection +# (default reqwest feature disabled by `default-features = false`). Lets operators route +# the native agent's LLM API calls (Anthropic / OpenAI / Bedrock-over-reqwest / etc.) +# and other reqwest HTTP calls through an HTTP CONNECT proxy by setting the env vars. +reqwest = { version = "0.12", default-features = false, features = ["rustls-tls", "json", "system-proxy"] } # rmcp 1.7 is on reqwest 0.13; its `StreamableHttpClient`/`AuthClient` impls are # for the 0.13 `Client`, so the OAuth transport must hold that type. The rest of -# the crate stays on 0.12. -reqwest013 = { package = "reqwest", version = "0.13", default-features = false, features = ["rustls", "json", "stream"] } +# the crate stays on 0.12. `system-proxy` re-enabled for the same reason as above so +# the OAuth transport also honors proxy env vars. +reqwest013 = { package = "reqwest", version = "0.13", default-features = false, features = ["rustls", "json", "stream", "system-proxy"] } anyhow = "1" async-trait = "0.1" uuid = { version = "1", features = ["v4"] } From 7b4bd4973602fec46a9bf64b0ef06d6f21850228 Mon Sep 17 00:00:00 2001 From: yen <5915590+antigenius0910@users.noreply.github.com> Date: Wed, 15 Jul 2026 05:19:40 -0500 Subject: [PATCH 8/8] =?UTF-8?q?fix(chart):=20address=20PR=20#1400=20round-?= =?UTF-8?q?7=20review=20=E2=80=94=20gateway=20env=20dedup,=20configToml=20?= =?UTF-8?q?recipe?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses both round-7 findings from chaodu-agent on commit 802cdd9. F1 (RUST_LOG duplicate in gateway env): templates/gateway.yaml emitted the built-in RUST_LOG entry unconditionally, then looped over gateway.env — so a user-supplied RUST_LOG produced two env entries with the same name. Since Kubernetes uses `name` as the merge key for pod env, this makes downstream precedence undefined and can be rejected by server-side-apply / GitOps tooling. Fix: check `hasKey $gwEnv "RUST_LOG"` before emitting the built-in. When user sets RUST_LOG via gateway.env, only their value is rendered. When not set, the built-in gateway.rustLog default (or "info") is emitted as before. Preserves backwards compatibility for the common case. Updated the round-2 gateway.env test to add two assertions the previous version missed: (a) user's RUST_LOG value=debug is present; (b) the built-in RUST_LOG value=info is NOT present. Added a companion positive test to prove the built-in still renders when user does not override. F2 (external-gateway recipe used the removed gateway.url value): The round-6 warning recipe told operators to set agents..gateway.url alongside gateway.deploy=false + extraRules. Since chart v0.10 the ConfigMap template only accepts agents..configToml or configUrl — the standalone gateway.url value is declared in values.yaml but no template consumes it. Following the recipe verbatim would trip the v0.10 migration fail-guard at templates/configmap.yaml:17. The round-6 test only rendered templates/networkpolicy.yaml so it validated the extraRules fragment but bypassed the full-chart contract. Fix (docs): - Rewrote the extraRules recipe in values.yaml to supply the gateway URL through a [gateway] section inside configToml, alongside gateway.deploy= false and the extraRules entry. Explicitly notes the v0.10 configToml/ configUrl requirement. - Chart README rows for `gateway.deploy` and `networkPolicy.egress. extraRules` mirror the corrected recipe and call out the configToml path change. Fix (tests): - New tests/networkpolicy_external_gateway_test.yaml suite renders BOTH templates/configmap.yaml AND templates/networkpolicy.yaml for the documented recipe. Kept as a separate suite because adding configmap.yaml to the main NP suite would force every existing test to also satisfy the v0.10 configToml requirement. - Case 1: ConfigMap accepts the recipe's configToml — proves the [gateway] section survives the v0.10 fail-guard and renders with the expected url/platform fields. - Case 2: NetworkPolicy renders the extraRules TCP 8080 entry and does NOT emit a chart-managed agent→gateway rule (deploy=false boundary). - Upgraded the round-6 in-suite external-gateway test to include the same configToml so it stays representative of the documented flow. Tests: 56/56 helm-unittest pass (was 53, +3: RUST_LOG dedup + RUST_LOG default + external-gateway full-chart pair). helm lint clean. Refs #1394 Refs #1400 round-7 review by chaodu-agent --- charts/openab/README.md | 4 +- charts/openab/templates/gateway.yaml | 5 +- .../networkpolicy_external_gateway_test.yaml | 94 +++++++++++++++++++ charts/openab/tests/networkpolicy_test.yaml | 43 ++++++++- charts/openab/values.yaml | 17 +++- 5 files changed, 153 insertions(+), 10 deletions(-) create mode 100644 charts/openab/tests/networkpolicy_external_gateway_test.yaml diff --git a/charts/openab/README.md b/charts/openab/README.md index f74479f2b..3ef08b09f 100644 --- a/charts/openab/README.md +++ b/charts/openab/README.md @@ -23,7 +23,7 @@ This page highlights commonly used values and deployment patterns. For the compl | `networkPolicy.egress.allowIpv6` | Add `::/0` to the HTTPS/HTTP rules alongside `0.0.0.0/0`, excluding CIDRs from `metadataExclusions.ipv6`. Turn off on IPv4-only clusters. | `true` | | `networkPolicy.egress.metadataExclusions.ipv4` | CIDRs excluded from the IPv4 HTTPS/HTTP allow rule. Defaults to `169.254.169.254/32` (AWS/GCP/Azure IMDS). Add IBM Cloud's `161.26.0.0/16` here if applicable. **NetworkPolicy allow rules combine additively — you cannot subtract via `extraRules`; edit this list directly.** ⚠️ **Defense-in-depth only, not a portable IMDS guarantee**: pods can always reach services on their resident node, and CNI implementations differ in whether `ipBlock` matches pre- or post-NAT addresses. Use cloud-provider hardening (IMDSv2 mandatory + hop-limit 1, GCP metadata concealment, Azure metadata endpoint policy) as the primary control. | `["169.254.169.254/32"]` | | `networkPolicy.egress.metadataExclusions.ipv6` | CIDRs excluded from the IPv6 HTTPS/HTTP allow rule. Defaults to `fd00:ec2::254/128` (AWS IPv6 IMDS). Non-AWS IPv6 metadata addresses differ — override this list per cloud. **Same additive-only semantics as `ipv4` above, and same defense-in-depth caveat.** | `["fd00:ec2::254/128"]` | -| `networkPolicy.egress.extraRules` | Raw `NetworkPolicyEgressRule` entries appended verbatim. Use to open extra ports or add cluster-specific carve-outs. **Cannot subtract from existing allows — for tighter metadata exclusions use `metadataExclusions` above.** ⚠️ **Non-80/443 destinations require an explicit entry here** — enabling `networkPolicy.enabled=true` silently breaks: (a) external config-only gateway (`gateway.deploy=false` + `gateway.url` on TCP 8080), (b) self-hosted STT on non-standard ports (e.g. `http://192.168.1.100:8080/v1`), (c) internal LLM proxies (LiteLLM on 4000, etc.). See `values.yaml` for a concrete `extraRules` recipe for the external-gateway case. | `[]` | +| `networkPolicy.egress.extraRules` | Raw `NetworkPolicyEgressRule` entries appended verbatim. Use to open extra ports or add cluster-specific carve-outs. **Cannot subtract from existing allows — for tighter metadata exclusions use `metadataExclusions` above.** ⚠️ **Non-80/443 destinations require an explicit entry here** — enabling `networkPolicy.enabled=true` silently breaks: (a) external config-only gateway (`gateway.deploy=false` + URL supplied via `configToml`'s `[gateway]` section on TCP 8080), (b) self-hosted STT on non-standard ports (e.g. `http://192.168.1.100:8080/v1`), (c) internal LLM proxies (LiteLLM on 4000, etc.). See `values.yaml` for a complete `extraRules` + `configToml` recipe for the external-gateway case. | `[]` | ### Agent values @@ -57,7 +57,7 @@ Each agent lives under `agents.`. | `stt.model` | STT model name. | `"whisper-large-v3-turbo"` | | `stt.baseUrl` | STT API base URL. | `"https://api.groq.com/openai/v1"` | | `gateway.enabled` | Enable the gateway config block for webhook-based platforms. | `false` | -| `gateway.deploy` | Deploy the gateway Deployment and Service. ⚠️ When `false` (config-only / external gateway), the chart-managed agent→gateway TCP 8080 egress rule is NOT rendered — if `networkPolicy.enabled=true` you must add a matching entry to `networkPolicy.egress.extraRules` targeting your external gateway pod/service, or agent WSS to the external gateway will be blocked. See values.yaml for a concrete recipe. | `true` | +| `gateway.deploy` | Deploy the gateway Deployment and Service. ⚠️ When `false` (config-only / external gateway), you must (a) supply the gateway URL via `configToml` (a `[gateway]` section — the standalone `gateway.url` value is not consumed by the ConfigMap since chart v0.10), and (b) if `networkPolicy.enabled=true`, add a matching `networkPolicy.egress.extraRules` entry targeting your external gateway pod/service, otherwise the agent→external-gateway WSS is blocked. See `values.yaml` for the complete recipe. | `true` | | `gateway.env` | Additional environment variables for the gateway container (map, `{name: value}`). Same shape as agent-level `env`. Useful for feature flags, `RUST_LOG` overrides, or HTTP proxy env vars if the gateway runs behind a transparent proxy. | `{}` | | `gateway.envFrom` | Load env vars for the gateway from existing Secrets or ConfigMaps (list of `secretRef`/`configMapRef` entries). Same shape as agent-level `envFrom`. | `[]` | | `cron.usercronEnabled` | Enable user-provided cron configuration. | `false` | diff --git a/charts/openab/templates/gateway.yaml b/charts/openab/templates/gateway.yaml index 026a69d38..3dda39b7b 100644 --- a/charts/openab/templates/gateway.yaml +++ b/charts/openab/templates/gateway.yaml @@ -218,9 +218,12 @@ spec: value: {{ ($cfg.gateway).wecom.debounceSecs | quote }} {{- end }} {{- end }} + {{- $gwEnv := ($cfg.gateway).env | default dict }} + {{- if not (hasKey $gwEnv "RUST_LOG") }} - name: RUST_LOG value: {{ ($cfg.gateway).rustLog | default "info" | quote }} - {{- range $k, $v := ($cfg.gateway).env }} + {{- end }} + {{- range $k, $v := $gwEnv }} - name: {{ $k }} value: {{ $v | quote }} {{- end }} diff --git a/charts/openab/tests/networkpolicy_external_gateway_test.yaml b/charts/openab/tests/networkpolicy_external_gateway_test.yaml new file mode 100644 index 000000000..58cd3cf9a --- /dev/null +++ b/charts/openab/tests/networkpolicy_external_gateway_test.yaml @@ -0,0 +1,94 @@ +suite: NetworkPolicy — external-gateway recipe full-chart guard (round-7 F2) +# This suite renders the ConfigMap AND NetworkPolicy together for the +# documented external-gateway extraRules recipe (chart README + values.yaml). +# It exists to catch config-schema migrations (e.g. the v0.10 configToml +# requirement) that would silently invalidate the recipe if the two +# templates were tested in isolation. +templates: + - templates/configmap.yaml + - templates/networkpolicy.yaml + +tests: + - it: ConfigMap accepts the recipe's configToml containing a [gateway] section + template: templates/configmap.yaml + set: + networkPolicy.enabled: true + networkPolicy.egress.extraRules: + - to: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: gateway-ns + podSelector: + matchLabels: + app: openab-gateway + ports: + - protocol: TCP + port: 8080 + agents.kiro.gateway.enabled: true + agents.kiro.gateway.deploy: false + agents.kiro.configToml: | + [gateway] + url = "ws://openab-gateway.gateway-ns.svc:8080/ws" + platform = "telegram" + allow_all_channels = true + asserts: + - hasDocuments: + count: 1 + - equal: + path: kind + value: ConfigMap + - matchRegex: + path: data["config.toml"] + pattern: 'url = "ws://openab-gateway\.gateway-ns\.svc:8080/ws"' + - matchRegex: + path: data["config.toml"] + pattern: 'platform = "telegram"' + + - it: NetworkPolicy renders the recipe's extraRules TCP 8080 entry and does NOT emit a chart-managed agent→gateway rule (gateway.deploy=false) + template: templates/networkpolicy.yaml + set: + networkPolicy.enabled: true + networkPolicy.egress.extraRules: + - to: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: gateway-ns + podSelector: + matchLabels: + app: openab-gateway + ports: + - protocol: TCP + port: 8080 + agents.kiro.gateway.enabled: true + agents.kiro.gateway.deploy: false + agents.kiro.configToml: | + [gateway] + url = "ws://openab-gateway.gateway-ns.svc:8080/ws" + platform = "telegram" + allow_all_channels = true + asserts: + - hasDocuments: + count: 1 + - contains: + path: spec.egress + content: + to: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: gateway-ns + podSelector: + matchLabels: + app: openab-gateway + ports: + - protocol: TCP + port: 8080 + - notContains: + path: spec.egress + content: + to: + - podSelector: + matchLabels: + app.kubernetes.io/name: openab + app.kubernetes.io/instance: RELEASE-NAME + app.kubernetes.io/component: kiro-gateway + any: true diff --git a/charts/openab/tests/networkpolicy_test.yaml b/charts/openab/tests/networkpolicy_test.yaml index 4f9a58cdc..ed786fb46 100644 --- a/charts/openab/tests/networkpolicy_test.yaml +++ b/charts/openab/tests/networkpolicy_test.yaml @@ -271,7 +271,13 @@ tests: - protocol: TCP port: 443 - - it: documented extraRules recipe for external-gateway TCP 8080 renders when gateway.deploy=false (round-6 F1) + # Round-7 F2: full-chart render for the external-gateway recipe — ensures + # both the ConfigMap (configToml with [gateway] section) AND the + # NetworkPolicy (extraRules for TCP 8080) render successfully together. + # A future config-schema migration that breaks the recipe would fail these + # two tests instead of silently invalidating documented guidance. + + - it: external-gateway recipe — NetworkPolicy renders the extraRules TCP 8080 entry and does NOT emit a chart-managed agent→gateway rule (round-6 F1 + round-7 F2) template: templates/networkpolicy.yaml set: networkPolicy.enabled: true @@ -288,6 +294,11 @@ tests: port: 8080 agents.kiro.gateway.enabled: true agents.kiro.gateway.deploy: false + agents.kiro.configToml: | + [gateway] + url = "ws://openab-gateway.gateway-ns.svc:8080/ws" + platform = "telegram" + allow_all_channels = true asserts: - hasDocuments: count: 1 @@ -304,7 +315,6 @@ tests: ports: - protocol: TCP port: 8080 - # No chart-managed agent→gateway rule (gateway.deploy=false) - notContains: path: spec.egress content: @@ -345,7 +355,7 @@ tests: # ── Gateway env / envFrom (general-purpose env injection) ───────────────── - - it: renders gateway.env into the gateway Deployment + - it: renders gateway.env into the gateway Deployment; user-supplied RUST_LOG wins over built-in and appears exactly once (round-7 F1) template: templates/gateway.yaml set: agents.kiro.gateway.enabled: true @@ -359,6 +369,33 @@ tests: name: MY_FLAG value: "on" documentIndex: 0 + - contains: + path: spec.template.spec.containers[0].env + content: + name: RUST_LOG + value: "openab_gateway=debug" + documentIndex: 0 + # RUST_LOG must appear exactly once — no built-in duplicate + - notContains: + path: spec.template.spec.containers[0].env + content: + name: RUST_LOG + value: "info" + documentIndex: 0 + + - it: emits built-in RUST_LOG when gateway.env does NOT override it (round-7 F1 default path) + template: templates/gateway.yaml + set: + agents.kiro.gateway.enabled: true + agents.kiro.gateway.env: + MY_FLAG: "on" + asserts: + - contains: + path: spec.template.spec.containers[0].env + content: + name: RUST_LOG + value: "info" + documentIndex: 0 - it: renders gateway.envFrom on the gateway Deployment template: templates/gateway.yaml diff --git a/charts/openab/values.yaml b/charts/openab/values.yaml index c6d8669de..a74a39585 100644 --- a/charts/openab/values.yaml +++ b/charts/openab/values.yaml @@ -138,11 +138,20 @@ networkPolicy: # breaks that traffic. Common cases: # # 1. External (config-only) gateway — agents..gateway.deploy=false - # with agents..gateway.url pointing at ws://openab-gateway:8080/ws - # does NOT get the chart-managed agent→gateway port-8080 exception - # (that only fires when gateway.deploy is true). Add: + # does NOT get the chart-managed agent→gateway TCP 8080 exception + # (that only fires when gateway.deploy is true). Since chart v0.10 the + # gateway URL must be supplied via configToml (or configUrl) — the + # standalone gateway.url value is no longer consumed by the ConfigMap. + # Complete recipe (agent-side values): # - # extraRules: + # agents..gateway.enabled: true + # agents..gateway.deploy: false + # agents..configToml: | + # [gateway] + # url = "ws://openab-gateway.gateway-ns.svc:8080/ws" + # platform = "telegram" + # allow_all_channels = true + # networkPolicy.egress.extraRules: # - to: # - namespaceSelector: # matchLabels: