Skip to content

feat(deploy): the chart can configure what the security page says it can, and the CORS setting reaches the server - #1533

Merged
diillson merged 1 commit into
mainfrom
feat/deploy-security-surface
Sep 5, 2026
Merged

feat(deploy): the chart can configure what the security page says it can, and the CORS setting reaches the server#1533
diillson merged 1 commit into
mainfrom
feat/deploy-security-surface

Conversation

@diillson

@diillson diillson commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Fifth PR from the features/security audit — the Kubernetes surface. Three settings the docs describe had no path from a values file to the running process.

The Helm JWT block in the docs does not exist

The page shows server.jwt.{secret,issuer,audience}. The chart has security.jwtSecret and security.jwtSecretRef, and jwtIssuer / jwtAudience exist in no chart. Leaving those unset skips the claim checks, so a deployment sharing a signing key across services had no way to say which audience it accepts.

Added: security.jwtIssuer, security.jwtAudience, security.jwtPublicKey (+ jwtPublicKeyRef for the RS256 key from PR #1528), and security.tlsClientCA for the mutual TLS from the same PR. values.schema.json updated alongside — the chart validates strictly, which is how I found out the keys were missing.

The NetworkPolicy could not be turned on

Not "off by default" — unrenderable. Its ingress block reads .Values.metrics.enabled, and the chart defines no metrics map:

$ helm template test deploy/helm/chatcli --set networkPolicy.enabled=true
Error: chatcli/templates/networkpolicy.yaml:19:18
  nil pointer evaluating interface {}.enabled

That reproduces on main with no changes of mine. The protection the page lists as Active was unreachable.

Fixed to read server.metricsPort, the value the deployment already passes to --metrics-port. And egress became a choice: allowAll (default, unchanged — the previous egress: - {}) or restricted, narrowing to DNS, HTTPS and the Kubernetes API, which is closer to the manifest the docs show. DNS is not optional in the narrow form; a pod that cannot resolve names fails in ways that look nothing like a policy problem. egressExtraPorts covers a private model endpoint.

The operator's CORS origin was read by nobody

CHATCLI_CORS_ORIGIN is injected into the pod by the chart. grep -rn "CHATCLI_CORS_ORIGIN\|SetCORSOrigin" --include='*.go' . finds only the setter's own definition — no caller anywhere. The API was deny-all regardless of configuration, so the dashboard could not call it from a browser however it was set up. Safe, but not the documented "CORS policy with configurable allowed origins".

The policy now reads the environment the chart was already setting, and grew what the docs promise: several origins, configurable methods, credentials. An allowlist echoes the request's own origin after matching, with Vary: Origin — the header carries one value, and echoing an unmatched origin would turn the list into "any site". A wildcard with credentials echoes the origin too, since browsers reject the literal * in that combination. Still deny-all until an origin is named, and the operator logs which policy took effect — the previous silence is how a setting stays broken for a long time.

Verification

6 CORS tests (deny-all default, allowlist matching, wildcard with and without credentials, preflight, and the legacy variable actually reaching the policy). helm lint passes on both charts; both NetworkPolicy modes render and were inspected. Root and operator suites green, go vet clean on both modules.

…can, and the CORS setting reaches the server

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.
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Quality Gate

Result: ✅ all floors passed

Floor Status Result Δ vs main Budget
1 · Build & Static go build / vet / fmt / lint
2 · Coverage 59.1% (bootstrap) 0 ≥ baseline
3 · Patch coverage 79.6% (req ≥ 60%) ≥ 60%
4 · AI smells diff scanned
5 · Scope budget ⚠️ 12 files / 746 LOC (code 746 + tooling 0) warn 800·25
6 · E2E go test -race ./e2e/... ≤ 15min
7 · Commit lint conventional commits
8 · Cyclo (new code) 4 file(s) under threshold ≤ 30
9 · Secrets scan gitleaks
10 · i18n parity missing 0, unknown 0
11 · CRD drift drifted: 0
12 · License headers 0 missing
13 · API breaking 0 incompatible
14 · Binary size chatcli 96.6MB · operator 54.3MB 100MB each
15 · Provider parity 15 providers · 0 violations

Config: .github/quality-gate.yml. Workflow: .github/workflows/quality-gate.yml.

@diillson
diillson merged commit e4d38c0 into main Sep 5, 2026
25 checks passed
@diillson
diillson deleted the feat/deploy-security-surface branch September 5, 2026 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant