From fe61ea96408d97a99ca5280691dc02b0d486ad6d Mon Sep 17 00:00:00 2001 From: diillson Date: Sat, 5 Sep 2026 16:38:16 -0300 Subject: [PATCH] feat(deploy): the chart can configure what the security page says it can, and the CORS setting reaches the server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three settings the documentation describes had no path from a values file to the running process. The chart could set a JWT secret and nothing else about JWT: no issuer, no audience, no public key. Leaving issuer and audience unset skips those checks, which means a token minted for another audience by the same issuer is accepted, so a deployment that shares a signing key had no way to say so. All four are settable now, the public key by reference to a Secret as well as inline, alongside the client CA bundle that turns TLS into mutual TLS. The NetworkPolicy could not be turned on at all. Its ingress block read a values map the chart does not define, so enabling it failed to render with a nil pointer — the protection the page lists as active was unreachable, not merely off. It renders now, from the metrics port the deployment actually uses, and egress became a choice: unrestricted as before, or narrowed to DNS, HTTPS and the Kubernetes API, which is what a network policy is usually installed for. DNS is not optional in the narrow form, because a pod that cannot resolve names fails in ways that look nothing like a policy problem. The operator's CORS origin was injected into the pod by the chart and read by nobody: the setter existed and no caller ever reached it, so the API was deny-all whatever anyone configured, and a dashboard could not call it from a browser. The policy now comes from the environment the chart was already setting, and grew what a real policy needs: several origins rather than one, configurable methods, and credentials. An allowlist echoes the request's own origin after matching it, with Vary on Origin, because the header carries a single value and echoing an unmatched one would turn the list into "any site". A wildcard alongside credentials echoes the origin too, since browsers reject the literal star in that combination. It is still deny-all until an origin is named, and the operator now logs which policy took effect, because the previous silence is how a setting stays broken for a long time. The rate-limit burst comment in the values file said 30, which stopped being true when the code moved to the documented 20. --- .../templates/deployment.yaml | 12 ++ .../helm/chatcli-operator/values.schema.json | 116 +++++++++-- deploy/helm/chatcli-operator/values.yaml | 8 +- deploy/helm/chatcli/templates/deployment.yaml | 23 +++ .../helm/chatcli/templates/networkpolicy.yaml | 34 +++- deploy/helm/chatcli/values.schema.json | 180 +++++++++++++++--- deploy/helm/chatcli/values.yaml | 38 +++- operator/api/rest/cors.go | 138 ++++++++++++++ operator/api/rest/cors_test.go | 131 +++++++++++++ operator/api/rest/middleware.go | 26 +-- operator/api/rest/server.go | 31 ++- operator/main.go | 9 + 12 files changed, 675 insertions(+), 71 deletions(-) create mode 100644 operator/api/rest/cors.go create mode 100644 operator/api/rest/cors_test.go diff --git a/deploy/helm/chatcli-operator/templates/deployment.yaml b/deploy/helm/chatcli-operator/templates/deployment.yaml index 2319e4b4..c2ee023f 100644 --- a/deploy/helm/chatcli-operator/templates/deployment.yaml +++ b/deploy/helm/chatcli-operator/templates/deployment.yaml @@ -91,6 +91,18 @@ spec: - name: CHATCLI_CORS_ORIGIN value: {{ .Values.security.corsOrigin | quote }} {{- end }} + {{- with .Values.security.corsAllowedOrigins }} + - name: CHATCLI_CORS_ALLOWED_ORIGINS + value: {{ join "," . | quote }} + {{- end }} + {{- with .Values.security.corsAllowedMethods }} + - name: CHATCLI_CORS_ALLOWED_METHODS + value: {{ join "," . | quote }} + {{- end }} + {{- if .Values.security.corsAllowCredentials }} + - name: CHATCLI_CORS_ALLOW_CREDENTIALS + value: "true" + {{- end }} {{- if .Values.security.auditLogPath }} - name: CHATCLI_AUDIT_LOG_PATH value: {{ .Values.security.auditLogPath | quote }} diff --git a/deploy/helm/chatcli-operator/values.schema.json b/deploy/helm/chatcli-operator/values.schema.json index 3c309231..fad37b1a 100644 --- a/deploy/helm/chatcli-operator/values.schema.json +++ b/deploy/helm/chatcli-operator/values.schema.json @@ -13,7 +13,9 @@ "image": { "type": "object", "description": "Container image configuration.", - "required": ["repository"], + "required": [ + "repository" + ], "additionalProperties": false, "properties": { "repository": { @@ -27,7 +29,11 @@ }, "pullPolicy": { "type": "string", - "enum": ["Always", "IfNotPresent", "Never"], + "enum": [ + "Always", + "IfNotPresent", + "Never" + ], "description": "Kubernetes image pull policy.", "default": "IfNotPresent" } @@ -38,7 +44,9 @@ "description": "List of Docker registry pull secrets.", "items": { "type": "object", - "required": ["name"], + "required": [ + "name" + ], "additionalProperties": false, "properties": { "name": { @@ -162,9 +170,26 @@ "description": "Container image for the kubectl-apply Job that ships with the hook.", "additionalProperties": false, "properties": { - "repository": {"type": "string", "description": "Image repository. Defaults to the official Kubernetes-project distroless kubectl image at registry.k8s.io.", "default": "registry.k8s.io/kubectl"}, - "tag": {"type": "string", "description": "Image tag. Must be a fully-qualified patch tag (e.g. v1.31.10) — major-minor stubs like v1.31 are NOT resolvable on registry.k8s.io.", "default": "v1.31.10"}, - "pullPolicy": {"type": "string", "enum": ["Always", "IfNotPresent", "Never"], "description": "Image pull policy.", "default": "IfNotPresent"} + "repository": { + "type": "string", + "description": "Image repository. Defaults to the official Kubernetes-project distroless kubectl image at registry.k8s.io.", + "default": "registry.k8s.io/kubectl" + }, + "tag": { + "type": "string", + "description": "Image tag. Must be a fully-qualified patch tag (e.g. v1.31.10) \u2014 major-minor stubs like v1.31 are NOT resolvable on registry.k8s.io.", + "default": "v1.31.10" + }, + "pullPolicy": { + "type": "string", + "enum": [ + "Always", + "IfNotPresent", + "Never" + ], + "description": "Image pull policy.", + "default": "IfNotPresent" + } } }, "resources": { @@ -175,12 +200,16 @@ "tolerations": { "type": "array", "description": "Tolerations for the hook Job pod.", - "items": {"type": "object"} + "items": { + "type": "object" + } }, "nodeSelector": { "type": "object", "description": "NodeSelector for the hook Job pod.", - "additionalProperties": {"type": "string"} + "additionalProperties": { + "type": "string" + } } } }, @@ -191,7 +220,11 @@ "properties": { "type": { "type": "string", - "enum": ["ClusterIP", "NodePort", "LoadBalancer"], + "enum": [ + "ClusterIP", + "NodePort", + "LoadBalancer" + ], "description": "Kubernetes Service type.", "default": "ClusterIP" } @@ -324,14 +357,21 @@ }, "operator": { "type": "string", - "enum": ["Exists", "Equal"] + "enum": [ + "Exists", + "Equal" + ] }, "value": { "type": "string" }, "effect": { "type": "string", - "enum": ["NoSchedule", "PreferNoSchedule", "NoExecute"] + "enum": [ + "NoSchedule", + "PreferNoSchedule", + "NoExecute" + ] }, "tolerationSeconds": { "type": "integer" @@ -388,17 +428,27 @@ "type": "object", "description": "TLS configuration for the REST API.", "properties": { - "certFile": { "type": "string" }, - "keyFile": { "type": "string" } + "certFile": { + "type": "string" + }, + "keyFile": { + "type": "string" + } } }, "grpcTLS": { "type": "object", "description": "TLS for gRPC communication with ChatCLI server.", "properties": { - "certFile": { "type": "string" }, - "keyFile": { "type": "string" }, - "caFile": { "type": "string" } + "certFile": { + "type": "string" + }, + "keyFile": { + "type": "string" + }, + "caFile": { + "type": "string" + } } }, "allowedResourceTypes": { @@ -416,6 +466,24 @@ "auditLogPath": { "type": "string", "description": "File path for structured audit logs." + }, + "corsAllowedOrigins": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Origins allowed to call the operator REST API from a browser." + }, + "corsAllowedMethods": { + "type": "array", + "items": { + "type": "string" + }, + "description": "HTTP methods allowed cross-origin; defaults to GET, POST, PUT, DELETE, OPTIONS." + }, + "corsAllowCredentials": { + "type": "boolean", + "description": "Allow cookies and Authorization on cross-origin requests." } } }, @@ -424,11 +492,19 @@ "description": "Extra environment variables for the operator pod.", "items": { "type": "object", - "required": ["name"], + "required": [ + "name" + ], "properties": { - "name": { "type": "string" }, - "value": { "type": "string" }, - "valueFrom": { "type": "object" } + "name": { + "type": "string" + }, + "value": { + "type": "string" + }, + "valueFrom": { + "type": "object" + } } } } diff --git a/deploy/helm/chatcli-operator/values.yaml b/deploy/helm/chatcli-operator/values.yaml index f9a6d5a5..d8f963ee 100644 --- a/deploy/helm/chatcli-operator/values.yaml +++ b/deploy/helm/chatcli-operator/values.yaml @@ -131,7 +131,13 @@ security: # Custom log scrub patterns (comma-separated regexes) logScrubPatterns: "" # CORS allowed origin (empty = deny all) - corsOrigin: "" + # CORS for the operator REST API (dashboard). Deny-all until an origin is + # named: with none, a browser blocks every cross-origin call. + corsOrigin: "" # a single origin (kept for compatibility) + corsAllowedOrigins: [] # several origins, or ["*"] for any + # - "https://dashboard.example.com" + corsAllowedMethods: [] # defaults to GET, POST, PUT, DELETE, OPTIONS + corsAllowCredentials: false # cookies / Authorization on cross-origin calls # Audit log file path auditLogPath: "" diff --git a/deploy/helm/chatcli/templates/deployment.yaml b/deploy/helm/chatcli/templates/deployment.yaml index 3e43e3f4..e5d67746 100644 --- a/deploy/helm/chatcli/templates/deployment.yaml +++ b/deploy/helm/chatcli/templates/deployment.yaml @@ -109,6 +109,29 @@ spec: name: {{ .Values.security.jwtSecretRef.name }} key: {{ .Values.security.jwtSecretRef.key }} {{- end }} + {{- if .Values.security.jwtIssuer }} + - name: CHATCLI_JWT_ISSUER + value: {{ .Values.security.jwtIssuer | quote }} + {{- end }} + {{- if .Values.security.jwtAudience }} + - name: CHATCLI_JWT_AUDIENCE + value: {{ .Values.security.jwtAudience | quote }} + {{- end }} + {{- if .Values.security.jwtPublicKey }} + - name: CHATCLI_JWT_PUBLIC_KEY + value: {{ .Values.security.jwtPublicKey | quote }} + {{- end }} + {{- if .Values.security.jwtPublicKeyRef }} + - name: CHATCLI_JWT_PUBLIC_KEY + valueFrom: + secretKeyRef: + name: {{ .Values.security.jwtPublicKeyRef.name }} + key: {{ .Values.security.jwtPublicKeyRef.key }} + {{- end }} + {{- if .Values.security.tlsClientCA }} + - name: CHATCLI_SERVER_TLS_CLIENT_CA + value: {{ .Values.security.tlsClientCA | quote }} + {{- end }} {{- if .Values.security.rateLimitRps }} - name: CHATCLI_RATE_LIMIT_RPS value: {{ .Values.security.rateLimitRps | quote }} diff --git a/deploy/helm/chatcli/templates/networkpolicy.yaml b/deploy/helm/chatcli/templates/networkpolicy.yaml index e5110a34..6c46032b 100644 --- a/deploy/helm/chatcli/templates/networkpolicy.yaml +++ b/deploy/helm/chatcli/templates/networkpolicy.yaml @@ -16,8 +16,15 @@ spec: - ports: - port: {{ .Values.server.port }} protocol: TCP - {{- if .Values.metrics.enabled }} - - port: {{ .Values.metrics.port | default 9090 }} + {{- /* + The metrics port comes from server.metricsPort, the same value the + deployment passes to --metrics-port. This block previously read a + .Values.metrics map that the chart does not define, so enabling the + policy at all failed to render with a nil pointer — the protection + could not be turned on. + */}} + {{- if .Values.server.metricsPort }} + - port: {{ .Values.server.metricsPort }} protocol: TCP {{- end }} {{- with .Values.networkPolicy.ingressFrom }} @@ -25,5 +32,28 @@ spec: {{- toYaml . | nindent 4 }} {{- end }} egress: + {{- if eq (.Values.networkPolicy.egress | default "allowAll") "restricted" }} + {{- /* + Restricted egress. DNS comes first and is not optional: a pod that + cannot resolve names fails in ways that look nothing like a network + policy problem, and every other rule here is written against names. + */}} + - ports: + - port: 53 + protocol: UDP + - port: 53 + protocol: TCP + - ports: + # HTTPS: LLM providers, and any other outbound API the server calls. + - port: 443 + protocol: TCP + # The Kubernetes API, for the watcher and the AIOps surface. + - port: {{ .Values.networkPolicy.kubernetesApiPort | default 6443 }} + protocol: TCP + {{- with .Values.networkPolicy.egressExtraPorts }} + {{- toYaml . | nindent 4 }} + {{- end }} + {{- else }} - {} # Allow all egress (LLM API calls, DNS, etc.) + {{- end }} {{- end }} diff --git a/deploy/helm/chatcli/values.schema.json b/deploy/helm/chatcli/values.schema.json index 59dc8302..954613c6 100644 --- a/deploy/helm/chatcli/values.schema.json +++ b/deploy/helm/chatcli/values.schema.json @@ -15,7 +15,9 @@ "type": "object", "description": "Container image configuration.", "additionalProperties": false, - "required": ["repository"], + "required": [ + "repository" + ], "properties": { "repository": { "type": "string", @@ -27,7 +29,11 @@ }, "pullPolicy": { "type": "string", - "enum": ["Always", "IfNotPresent", "Never"], + "enum": [ + "Always", + "IfNotPresent", + "Never" + ], "default": "IfNotPresent", "description": "Image pull policy." } @@ -200,7 +206,10 @@ }, "transport": { "type": "string", - "enum": ["stdio", "sse"], + "enum": [ + "stdio", + "sse" + ], "description": "Transport type for the MCP server." }, "command": { @@ -394,7 +403,10 @@ "items": { "type": "object", "additionalProperties": false, - "required": ["deployment", "namespace"], + "required": [ + "deployment", + "namespace" + ], "properties": { "deployment": { "type": "string", @@ -501,9 +513,26 @@ "description": "Container image for the kubectl-apply Job that ships with the hook.", "additionalProperties": false, "properties": { - "repository": {"type": "string", "description": "Image repository. Defaults to the official Kubernetes-project distroless kubectl image at registry.k8s.io.", "default": "registry.k8s.io/kubectl"}, - "tag": {"type": "string", "description": "Image tag. Must be a fully-qualified patch tag (e.g. v1.31.10) — major-minor stubs like v1.31 are NOT resolvable on registry.k8s.io.", "default": "v1.31.10"}, - "pullPolicy": {"type": "string", "enum": ["Always", "IfNotPresent", "Never"], "description": "Image pull policy.", "default": "IfNotPresent"} + "repository": { + "type": "string", + "description": "Image repository. Defaults to the official Kubernetes-project distroless kubectl image at registry.k8s.io.", + "default": "registry.k8s.io/kubectl" + }, + "tag": { + "type": "string", + "description": "Image tag. Must be a fully-qualified patch tag (e.g. v1.31.10) \u2014 major-minor stubs like v1.31 are NOT resolvable on registry.k8s.io.", + "default": "v1.31.10" + }, + "pullPolicy": { + "type": "string", + "enum": [ + "Always", + "IfNotPresent", + "Never" + ], + "description": "Image pull policy.", + "default": "IfNotPresent" + } } }, "resources": { @@ -514,12 +543,16 @@ "tolerations": { "type": "array", "description": "Tolerations for the hook Job pod.", - "items": {"type": "object"} + "items": { + "type": "object" + } }, "nodeSelector": { "type": "object", "description": "NodeSelector for the hook Job pod.", - "additionalProperties": {"type": "string"} + "additionalProperties": { + "type": "string" + } } } }, @@ -530,7 +563,11 @@ "properties": { "type": { "type": "string", - "enum": ["ClusterIP", "NodePort", "LoadBalancer"], + "enum": [ + "ClusterIP", + "NodePort", + "LoadBalancer" + ], "description": "Service type." }, "port": { @@ -701,15 +738,23 @@ }, "minAvailable": { "oneOf": [ - { "type": "integer" }, - { "type": "string" } + { + "type": "integer" + }, + { + "type": "string" + } ], "description": "Minimum number or percentage of pods that must be available." }, "maxUnavailable": { "oneOf": [ - { "type": "integer" }, - { "type": "string" } + { + "type": "integer" + }, + { + "type": "string" + } ], "description": "Maximum number or percentage of pods that can be unavailable." } @@ -723,6 +768,34 @@ "enabled": { "type": "boolean", "description": "Enable network policy." + }, + "egress": { + "type": "string", + "enum": [ + "allowAll", + "restricted" + ], + "description": "Egress policy: allowAll keeps outbound traffic unrestricted (default); restricted narrows it to DNS, HTTPS and the Kubernetes API." + }, + "kubernetesApiPort": { + "type": "integer", + "description": "Kubernetes API port allowed when egress is restricted." + }, + "egressExtraPorts": { + "type": "array", + "description": "Extra ports allowed on egress when egress is restricted.", + "items": { + "type": "object", + "properties": { + "port": { + "type": "integer" + }, + "protocol": { + "type": "string" + } + }, + "additionalProperties": false + } } } }, @@ -939,28 +1012,47 @@ "type": "object", "description": "Reference to a Kubernetes Secret key containing the JWT secret.", "properties": { - "name": { "type": "string" }, - "key": { "type": "string" } + "name": { + "type": "string" + }, + "key": { + "type": "string" + } } }, "rateLimitRps": { - "type": ["string", "integer"], + "type": [ + "string", + "integer" + ], "description": "Per-client rate limit in requests/second. Default: 10." }, "rateLimitBurst": { - "type": ["string", "integer"], + "type": [ + "string", + "integer" + ], "description": "Rate limit burst size. Default: 30." }, "maxRecvMsgSize": { - "type": ["string", "integer"], + "type": [ + "string", + "integer" + ], "description": "Max gRPC receive message size in bytes. Default: 52428800 (50MB)." }, "maxSendMsgSize": { - "type": ["string", "integer"], + "type": [ + "string", + "integer" + ], "description": "Max gRPC send message size in bytes. Default: 52428800 (50MB)." }, "maxConcurrentStreams": { - "type": ["string", "integer"], + "type": [ + "string", + "integer" + ], "description": "Max concurrent gRPC streams. Default: 100." }, "bindAddress": { @@ -977,16 +1069,27 @@ }, "agentSecurityMode": { "type": "string", - "enum": ["strict", "permissive", ""], + "enum": [ + "strict", + "permissive", + "" + ], "description": "Agent command validation mode. Default: strict." }, "sessionTTL": { - "type": ["string", "integer"], + "type": [ + "string", + "integer" + ], "description": "Session TTL in days. Default: 90." }, "envRedactMode": { "type": "string", - "enum": ["strict", "permissive", ""], + "enum": [ + "strict", + "permissive", + "" + ], "description": "Environment variable redaction mode. Default: permissive." }, "allowUnsignedPlugins": { @@ -1000,6 +1103,35 @@ "encryptionKey": { "type": "string", "description": "Encryption key for sessions at rest. Use secretKeyRef via extraEnv for production." + }, + "jwtIssuer": { + "type": "string", + "description": "Expected JWT \"iss\" claim. Empty skips the check." + }, + "jwtAudience": { + "type": "string", + "description": "Expected JWT \"aud\" claim. Empty skips the check." + }, + "jwtPublicKey": { + "type": "string", + "description": "RSA public key (PEM) or path to one; setting it selects RS256." + }, + "tlsClientCA": { + "type": "string", + "description": "Path to the CA bundle client certificates are verified against (mutual TLS). Requires tls.cert and tls.key." + }, + "jwtPublicKeyRef": { + "type": "object", + "description": "Secret reference holding the RSA public key.", + "properties": { + "name": { + "type": "string" + }, + "key": { + "type": "string" + } + }, + "additionalProperties": false } } } diff --git a/deploy/helm/chatcli/values.yaml b/deploy/helm/chatcli/values.yaml index 8e6edb6a..db756548 100644 --- a/deploy/helm/chatcli/values.yaml +++ b/deploy/helm/chatcli/values.yaml @@ -272,6 +272,23 @@ networkPolicy: enabled: false # ingressFrom: [] + # Egress policy. "allowAll" (the default) keeps the previous behaviour: + # the policy restricts ingress and leaves outbound traffic alone. + # "restricted" narrows egress to what the server actually needs — DNS, + # HTTPS for LLM providers, and the Kubernetes API — which is what a + # NetworkPolicy is usually installed for. + # + # Start with allowAll, confirm ingress behaves, then tighten. A pod that + # cannot resolve DNS fails in ways that look nothing like a network + # policy problem. + egress: allowAll # allowAll | restricted + # Extra ports to allow on egress when egress=restricted, for a private + # model endpoint or an internal service the server calls. + # egressExtraPorts: + # - port: 8080 + # protocol: TCP + kubernetesApiPort: 6443 + nodeSelector: {} tolerations: [] affinity: {} @@ -300,9 +317,28 @@ security: # name: chatcli-jwt # key: secret + # Expected JWT claims. Leaving either empty skips that check, which means + # a token minted for another audience by the same issuer is accepted — + # set both wherever more than one service shares a signing key. + jwtIssuer: "" # expected "iss" claim + jwtAudience: "" # expected "aud" claim + + # RS256: an RSA public key (PEM) or the path to one inside the container. + # Setting this selects RS256; leave it empty for an HS256 shared secret. + jwtPublicKey: "" + # Reference a Secret key holding the RSA public key (recommended) + jwtPublicKeyRef: {} + # name: chatcli-jwt + # key: public.pem + + # Mutual TLS: path to the CA bundle client certificates are verified + # against. Requires tls.cert and tls.key — there is no handshake to carry + # a client certificate without them. Mount the bundle via extraVolumes. + tlsClientCA: "" + # Per-client rate limiting rateLimitRps: "" # requests/second (default: 10) - rateLimitBurst: "" # burst size (default: 30) + rateLimitBurst: "" # burst size (default: 20) # gRPC message size limits maxRecvMsgSize: "" # bytes (default: 52428800 = 50MB) diff --git a/operator/api/rest/cors.go b/operator/api/rest/cors.go new file mode 100644 index 00000000..e2cd09da --- /dev/null +++ b/operator/api/rest/cors.go @@ -0,0 +1,138 @@ +/* + * ChatCLI - Command Line Interface for LLM interaction + * Copyright (c) 2024 Edilson Freitas + * License: Apache-2.0 + */ +package rest + +import ( + "net/http" + "os" + "strings" +) + +// CORSPolicy is the operator API's cross-origin configuration. +// +// It stays deny-all until an origin is named. What changed is that naming +// one now has an effect: the variable the Helm chart has been setting all +// along was read by nobody, so the dashboard could not call the API from a +// browser no matter how it was configured. +type CORSPolicy struct { + // AllowedOrigins is the exact set of origins allowed. "*" allows any, + // and is refused together with AllowCredentials. + AllowedOrigins []string + // AllowedMethods defaults to the verbs the API actually serves. + AllowedMethods []string + // AllowCredentials permits cookies and Authorization on cross-origin + // requests. + AllowCredentials bool +} + +var defaultCORSMethods = []string{"GET", "POST", "PUT", "DELETE", "OPTIONS"} + +// CORSPolicyFromEnv reads the policy from the environment. +// +// CHATCLI_CORS_ALLOWED_ORIGINS comma-separated list, or "*" +// CHATCLI_CORS_ORIGIN a single origin (kept: the chart has +// been setting it since before the list +// existed) +// CHATCLI_CORS_ALLOWED_METHODS comma-separated verbs +// CHATCLI_CORS_ALLOW_CREDENTIALS "true" to allow credentials +func CORSPolicyFromEnv() CORSPolicy { + p := CORSPolicy{ + AllowedOrigins: splitList(os.Getenv("CHATCLI_CORS_ALLOWED_ORIGINS")), + AllowedMethods: splitList(os.Getenv("CHATCLI_CORS_ALLOWED_METHODS")), + AllowCredentials: strings.EqualFold(strings.TrimSpace(os.Getenv("CHATCLI_CORS_ALLOW_CREDENTIALS")), "true"), + } + if single := strings.TrimSpace(os.Getenv("CHATCLI_CORS_ORIGIN")); single != "" { + p.AllowedOrigins = append(p.AllowedOrigins, single) + } + if len(p.AllowedMethods) == 0 { + p.AllowedMethods = defaultCORSMethods + } + return p +} + +func splitList(v string) []string { + var out []string + for _, part := range strings.Split(v, ",") { + if part = strings.TrimSpace(part); part != "" { + out = append(out, part) + } + } + return out +} + +// enabled reports whether any origin is allowed. +func (p CORSPolicy) enabled() bool { return len(p.AllowedOrigins) > 0 } + +// wildcard reports whether the policy allows any origin. +func (p CORSPolicy) wildcard() bool { + for _, o := range p.AllowedOrigins { + if o == "*" { + return true + } + } + return false +} + +// originFor returns the value to echo in Access-Control-Allow-Origin for a +// request, or "" when the request's origin is not allowed. +// +// An allowlist of several origins cannot be expressed in the header, which +// carries one value: the request's own origin is echoed back after being +// matched, and Vary: Origin tells caches the response depends on it. +// Echoing an unmatched origin would turn the allowlist into "any site". +func (p CORSPolicy) originFor(requestOrigin string) string { + if requestOrigin == "" { + return "" + } + for _, allowed := range p.AllowedOrigins { + if allowed == requestOrigin { + return requestOrigin + } + } + if p.wildcard() { + // With credentials, "*" is not a legal value and browsers reject + // the response; echoing the origin is the only working form. + if p.AllowCredentials { + return requestOrigin + } + return "*" + } + return "" +} + +// apply writes the CORS headers for a request, and reports whether the +// request is a preflight that has been answered. +func (p CORSPolicy) apply(w http.ResponseWriter, r *http.Request, apiKeyHeader string) (handled bool) { + if !p.enabled() { + return false + } + + // The response differs by origin whenever the allowlist is not a bare + // wildcard, so a shared cache must not serve one origin's response to + // another. + w.Header().Add("Vary", "Origin") + + allow := p.originFor(r.Header.Get("Origin")) + if allow == "" { + return false + } + + w.Header().Set("Access-Control-Allow-Origin", allow) + w.Header().Set("Access-Control-Allow-Methods", strings.Join(p.AllowedMethods, ", ")) + w.Header().Set("Access-Control-Allow-Headers", "Content-Type, "+apiKeyHeader+", Authorization") + if p.AllowCredentials { + w.Header().Set("Access-Control-Allow-Credentials", "true") + } else { + w.Header().Set("Access-Control-Allow-Credentials", "false") + } + w.Header().Set("Access-Control-Max-Age", "3600") // 1 hour, not 24h + + if r.Method == http.MethodOptions { + w.WriteHeader(http.StatusNoContent) + return true + } + return false +} diff --git a/operator/api/rest/cors_test.go b/operator/api/rest/cors_test.go new file mode 100644 index 00000000..f68db1ed --- /dev/null +++ b/operator/api/rest/cors_test.go @@ -0,0 +1,131 @@ +/* + * ChatCLI - Command Line Interface for LLM interaction + * Copyright (c) 2024 Edilson Freitas + * License: Apache-2.0 + */ +package rest + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +func applyPolicy(t *testing.T, p CORSPolicy, method, origin string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(method, "/api/v1/incidents", nil) + if origin != "" { + req.Header.Set("Origin", origin) + } + rec := httptest.NewRecorder() + p.apply(rec, req, "X-API-Key") + return rec +} + +// Deny-all stays the default: no origin configured, no headers written. +func TestCORS_DisabledByDefault(t *testing.T) { + rec := applyPolicy(t, CORSPolicy{}, http.MethodGet, "https://dashboard.example.com") + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("headers written with no policy configured: %q", got) + } +} + +// The allowlist echoes the request's own origin, because the header carries +// one value and echoing an unmatched one would allow any site. +func TestCORS_EchoesOnlyAllowedOrigins(t *testing.T) { + p := CORSPolicy{ + AllowedOrigins: []string{"https://a.example.com", "https://b.example.com"}, + AllowedMethods: defaultCORSMethods, + } + + for _, origin := range p.AllowedOrigins { + rec := applyPolicy(t, p, http.MethodGet, origin) + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != origin { + t.Errorf("allowed origin %q got %q", origin, got) + } + if rec.Header().Get("Vary") != "Origin" { + t.Errorf("Vary: Origin missing for %q", origin) + } + } + + rec := applyPolicy(t, p, http.MethodGet, "https://evil.example.com") + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "" { + t.Errorf("an origin outside the allowlist was echoed: %q", got) + } +} + +func TestCORS_WildcardAndCredentials(t *testing.T) { + plain := CORSPolicy{AllowedOrigins: []string{"*"}, AllowedMethods: defaultCORSMethods} + rec := applyPolicy(t, plain, http.MethodGet, "https://anywhere.example.com") + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "*" { + t.Errorf("wildcard without credentials should send *, got %q", got) + } + + // "*" is not a legal value alongside credentials; browsers reject it, + // so the origin has to be echoed instead. + withCreds := CORSPolicy{AllowedOrigins: []string{"*"}, AllowedMethods: defaultCORSMethods, AllowCredentials: true} + rec = applyPolicy(t, withCreds, http.MethodGet, "https://anywhere.example.com") + if got := rec.Header().Get("Access-Control-Allow-Origin"); got != "https://anywhere.example.com" { + t.Errorf("wildcard with credentials should echo the origin, got %q", got) + } + if got := rec.Header().Get("Access-Control-Allow-Credentials"); got != "true" { + t.Errorf("Access-Control-Allow-Credentials = %q", got) + } +} + +func TestCORS_PreflightIsAnswered(t *testing.T) { + p := CORSPolicy{AllowedOrigins: []string{"https://a.example.com"}, AllowedMethods: defaultCORSMethods} + req := httptest.NewRequest(http.MethodOptions, "/api/v1/incidents", nil) + req.Header.Set("Origin", "https://a.example.com") + rec := httptest.NewRecorder() + + if handled := p.apply(rec, req, "X-API-Key"); !handled { + t.Fatal("preflight was not answered") + } + if rec.Code != http.StatusNoContent { + t.Errorf("preflight status = %d, want 204", rec.Code) + } +} + +// The variable the chart has been setting all along must keep working, and +// must now actually reach the policy. +func TestCORS_ReadsTheEnvironmentTheChartSets(t *testing.T) { + t.Setenv("CHATCLI_CORS_ORIGIN", "https://legacy.example.com") + t.Setenv("CHATCLI_CORS_ALLOWED_ORIGINS", "") + t.Setenv("CHATCLI_CORS_ALLOWED_METHODS", "") + t.Setenv("CHATCLI_CORS_ALLOW_CREDENTIALS", "") + + p := CORSPolicyFromEnv() + if len(p.AllowedOrigins) != 1 || p.AllowedOrigins[0] != "https://legacy.example.com" { + t.Fatalf("CHATCLI_CORS_ORIGIN did not reach the policy: %+v", p.AllowedOrigins) + } + if len(p.AllowedMethods) == 0 { + t.Error("methods should fall back to the default verbs") + } + + t.Setenv("CHATCLI_CORS_ALLOWED_ORIGINS", "https://a.example.com, https://b.example.com") + t.Setenv("CHATCLI_CORS_ALLOWED_METHODS", "GET,POST") + t.Setenv("CHATCLI_CORS_ALLOW_CREDENTIALS", "true") + p = CORSPolicyFromEnv() + if len(p.AllowedOrigins) != 3 { + t.Errorf("list and single origin should compose: %+v", p.AllowedOrigins) + } + if len(p.AllowedMethods) != 2 { + t.Errorf("methods = %+v", p.AllowedMethods) + } + if !p.AllowCredentials { + t.Error("credentials flag did not reach the policy") + } +} + +func TestCORS_SetCORSOriginStillWorks(t *testing.T) { + s := &APIServer{apiKeyHeader: "X-API-Key"} + s.SetCORSOrigin("https://a.example.com") + if got := s.CORSAllowedOrigins(); len(got) != 1 || got[0] != "https://a.example.com" { + t.Fatalf("SetCORSOrigin = %+v", got) + } + s.SetCORSOrigin("") + if got := s.CORSAllowedOrigins(); len(got) != 0 { + t.Fatalf("clearing the origin left %+v", got) + } +} diff --git a/operator/api/rest/middleware.go b/operator/api/rest/middleware.go index 26813aa4..9362ce18 100644 --- a/operator/api/rest/middleware.go +++ b/operator/api/rest/middleware.go @@ -163,25 +163,15 @@ func (s *APIServer) rateLimitMiddleware(next http.Handler) http.Handler { // Security (H6): Default to deny-all CORS. Require explicit origin configuration. func (s *APIServer) corsMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - origin := s.corsOrigin - // Security: deny-all by default — CORS only if explicitly configured - if origin == "" { - // No CORS headers set — browser cross-origin requests will be blocked - next.ServeHTTP(w, r) - return - } - - w.Header().Set("Access-Control-Allow-Origin", origin) - w.Header().Set("Access-Control-Allow-Methods", "GET, POST, PUT, DELETE, OPTIONS") - w.Header().Set("Access-Control-Allow-Headers", "Content-Type, "+s.apiKeyHeader+", Authorization") - w.Header().Set("Access-Control-Allow-Credentials", "false") - w.Header().Set("Access-Control-Max-Age", "3600") // 1 hour, not 24h - - if r.Method == http.MethodOptions { - w.WriteHeader(http.StatusNoContent) - return + // Deny-all until an origin is configured: with no policy, no CORS + // headers are written and a browser blocks the cross-origin call. + s.corsMu.RLock() + policy := s.corsPolicy + s.corsMu.RUnlock() + + if policy.apply(w, r, s.apiKeyHeader) { + return // preflight answered } - next.ServeHTTP(w, r) }) } diff --git a/operator/api/rest/server.go b/operator/api/rest/server.go index 2c7af5fc..9eda53ee 100644 --- a/operator/api/rest/server.go +++ b/operator/api/rest/server.go @@ -86,7 +86,8 @@ type APIServer struct { apiKeysMu sync.RWMutex apiKeys map[string]string // key -> role limiter *rateLimiter - corsOrigin string + corsMu sync.RWMutex + corsPolicy CORSPolicy watcherBridge WatcherDedupInvalidator // optional, for dedup invalidation on manual resolve } @@ -101,8 +102,8 @@ func NewAPIServer(c client.Client, addr string) *APIServer { listenAddr: addr, apiKeyHeader: authHeaderName, apiKeys: make(map[string]string), - limiter: newRateLimiter(30), // Security (M3): 30 requests/minute (reduced from 100) - corsOrigin: "", // Security (H6): deny-all CORS by default + limiter: newRateLimiter(30), // Security (M3): 30 requests/minute (reduced from 100) + corsPolicy: CORSPolicyFromEnv(), // Security (H6): deny-all CORS unless an origin is configured } } @@ -119,9 +120,29 @@ func (s *APIServer) SetWatcherBridge(wb WatcherDedupInvalidator) { s.watcherBridge = wb } -// SetCORSOrigin configures the allowed CORS origin. +// SetCORSOrigin configures a single allowed CORS origin. func (s *APIServer) SetCORSOrigin(origin string) { - s.corsOrigin = origin + s.SetCORSPolicy(CORSPolicy{ + AllowedOrigins: splitList(origin), + AllowedMethods: defaultCORSMethods, + }) +} + +// SetCORSPolicy replaces the cross-origin policy. +func (s *APIServer) SetCORSPolicy(policy CORSPolicy) { + if len(policy.AllowedMethods) == 0 { + policy.AllowedMethods = defaultCORSMethods + } + s.corsMu.Lock() + defer s.corsMu.Unlock() + s.corsPolicy = policy +} + +// CORSAllowedOrigins reports the configured origins, for the startup log. +func (s *APIServer) CORSAllowedOrigins() []string { + s.corsMu.RLock() + defer s.corsMu.RUnlock() + return append([]string(nil), s.corsPolicy.AllowedOrigins...) } // Start implements manager.Runnable and starts the HTTP server. diff --git a/operator/main.go b/operator/main.go index 7a186841..d28b106c 100644 --- a/operator/main.go +++ b/operator/main.go @@ -255,6 +255,15 @@ func main() { aiopsPort = "8090" } apiServer := rest.NewAPIServer(mgr.GetClient(), ":"+aiopsPort) + // The CORS policy comes from the environment the chart already sets. + // Logged because it was previously read by nobody: an operator who + // configured an origin and saw the dashboard blocked had no way to tell + // whether the setting had arrived. + if origins := apiServer.CORSAllowedOrigins(); len(origins) > 0 { + setupLog.Info("CORS enabled", "allowedOrigins", origins) + } else { + setupLog.Info("CORS disabled (no allowed origin configured); browser cross-origin requests are blocked") + } // Load API keys from ConfigMap chatcli-operator-config (field: api-keys) // and start a watcher to hot-reload on changes (no restart needed)