Skip to content

fix(auth): enforce management API JWT audience - #182

Open
jsell-rh wants to merge 8 commits into
mainfrom
fix/api-jwt-audience-validation
Open

fix(auth): enforce management API JWT audience#182
jsell-rh wants to merge 8 commits into
mainfrom
fix/api-jwt-audience-validation

Conversation

@jsell-rh

@jsell-rh jsell-rh commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • upgrade the API server to the rh-trex-ai issuer/audience validation merged in fix(auth): validate JWT issuer and audience rh-trex-ai#52 and the fail-closed verification hardening merged in fix(auth): fail closed without JWT verification keys rh-trex-ai#53
  • require the configured Keycloak issuer and the hypershell-frontend management API audience in Kind and OpenShift deployments
  • document that aud identifies the resource while azp identifies the calling client, so hsctl, frontend, and control-plane clients can share the management API audience
  • reject dynamic gateway-client tokens that are not minted for the management API

Security hardening

The pinned rh-trex-ai commit rejects JWTs unless cryptographic verification marked them valid and makes gRPC authentication fail closed when no JWK provider is available.

Testing

  • GOTOOLCHAIN=auto go test ./pkg/... ./cmd/...
  • GOTOOLCHAIN=auto go test -race ./pkg/... ./cmd/...
  • GOTOOLCHAIN=auto go build ./cmd/hypershell
  • make check
  • make lint-api-server
  • go mod verify
  • kustomize build deploy/kind
  • kustomize build deploy/openshift

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 1b796b50-c8a8-43a1-b1d6-ced08274593c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@jsell-rh
jsell-rh enabled auto-merge August 21, 2026 21:08
@jsell-rh

jsell-rh commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Amber review

Status: Complete

Verdict

COMMENT — This is a well-scoped, correct security hardening that adds issuer/audience JWT validation on top of the existing JWKS signature check, with matching spec scenarios. No blocking issues on the HyperShell side; the Kind realm config is internally consistent, and the only findings are Minor (config symmetry, production rollout coordination, and a large transitive dependency footprint from the framework bump).

Amber Analysis

The PR bumps rh-trex-ai to a commit that enforces issuer/audience validation and fail-closed signed-token verification, then wires --jwt-issuer and --jwt-audience=hypershell-frontend into the Kind and OpenShift overlays and documents the aud (resource) vs azp (client) split in oidc-integration.spec.md. The design is coherent: in Kind, deploy/base/keycloak/keycloak.yaml already gives hypershell-frontend, hypershell-control-plane (L204-219), and the provisioner an audience mapper that emits hypershell-frontend, so control-plane gRPC writes and browser tokens keep working after enforcement is enabled. The Kind issuer value (https://keycloak.hypershell.localhost/realms/hypershell) correctly matches the BFF OIDC_ISSUER, and the overlay comment correctly explains why iss (external realm URL) is kept separate from the in-cluster JWKS route.

Confidence: High on the HyperShell-side correctness; Medium on production behavior, since the actual validation logic and the external SSO audience mappers live outside this diff.

Findings

1. [Minor] --jwt-audience is a hardcoded literal while issuer/JWKS come from the secretConfig vs Code (deploy/openshift/kustomization.yaml L107, deploy/kind/kustomization.yaml L139)

--jwk-cert-url and --jwt-issuer are sourced from hypershell-api-config, but the audience is baked into the manifest as hypershell-frontend. That is acceptable if the management-API resource audience is intentionally a fixed platform constant, but it is asymmetric with the other two values and would require a manifest edit (not just a secret change) if a deployment ever renamed the audience. Consider sourcing it from the same secret for symmetry, or add a one-line note that the audience is deliberately a fixed constant.

2. [Minor] Production rollout depends on external SSO emitting aud=hypershell-frontend for first-party callersRollout Coordination (deploy/openshift/kustomization.yaml L84-107)

Once this overlay applies, every non-bypassed HTTP/gRPC call (including the control plane's UpdateGateway, which is not in --auth-bypass-methods, and hsctl) must present a token whose aud contains hypershell-frontend. RBAC_SERVICE_ACCOUNTS bypasses the RBAC interceptor but not JWT authentication, so audience enforcement still applies to the control plane. Kind is safe because its realm config already has the mappers, but the OpenShift/external-SSO path is not covered by this diff. The overlay comment block (L63-70) already flags the RBAC breaking change; please extend it to state that the SSO must emit hypershell-frontend in aud for the control-plane and CLI clients before/with this rollout, or those callers will start returning 401 immediately after upgrade.

3. [Minor] Framework bump pulls a large TUI transitive footprint into the API server moduleDependency Hygiene (components/api-server/go.mod L40-125)

The rh-trex-ai bump adds a sizable set of indirect Charmbracelet/TUI dependencies (bubbletea, lipgloss, bubbles, termenv, go-runewidth, etc.) that are unrelated to JWT validation. This is inherited from upstream and not caused by this PR, but it is worth confirming these are intended upstream additions and that none of the new recent transitive modules trip the dependency-age policy (the allowlist only pins rh-trex-ai directly). make check reportedly passes, so this is informational.

Test Diff Scrutiny

No test files are modified in this PR, and no existing assertions were flipped. The behavioral change (audience/issuer now required) is enforced in the upstream rh-trex-ai module and is expressed here as new spec scenarios (oidc-integration.spec.md: "Token Issued for Another Resource Is Rejected") rather than modified in-repo tests. No removed-guarantee pattern detected.

@jsell-rh jsell-rh left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

COMMENT — This is a well-scoped, correct security hardening that adds issuer/audience JWT validation on top of the existing JWKS signature check, with matching spec scenarios. No blocking issues on the HyperShell side; the Kind realm config is internally consistent, and the only findings are Minor (config symmetry, production rollout coordination, and a large transitive dependency footprint from the framework bump).

Amber Analysis

The PR bumps rh-trex-ai to a commit that enforces issuer/audience validation and fail-closed signed-token verification, then wires --jwt-issuer and --jwt-audience=hypershell-frontend into the Kind and OpenShift overlays and documents the aud (resource) vs azp (client) split in oidc-integration.spec.md. The design is coherent: in Kind, deploy/base/keycloak/keycloak.yaml already gives hypershell-frontend, hypershell-control-plane (L204-219), and the provisioner an audience mapper that emits hypershell-frontend, so control-plane gRPC writes and browser tokens keep working after enforcement is enabled. The Kind issuer value (https://keycloak.hypershell.localhost/realms/hypershell) correctly matches the BFF OIDC_ISSUER, and the overlay comment correctly explains why iss (external realm URL) is kept separate from the in-cluster JWKS route.

Confidence: High on the HyperShell-side correctness; Medium on production behavior, since the actual validation logic and the external SSO audience mappers live outside this diff.

Findings

1. [Minor] --jwt-audience is a hardcoded literal while issuer/JWKS come from the secretConfig vs Code (deploy/openshift/kustomization.yaml L107, deploy/kind/kustomization.yaml L139)

--jwk-cert-url and --jwt-issuer are sourced from hypershell-api-config, but the audience is baked into the manifest as hypershell-frontend. That is acceptable if the management-API resource audience is intentionally a fixed platform constant, but it is asymmetric with the other two values and would require a manifest edit (not just a secret change) if a deployment ever renamed the audience. Consider sourcing it from the same secret for symmetry, or add a one-line note that the audience is deliberately a fixed constant.

2. [Minor] Production rollout depends on external SSO emitting aud=hypershell-frontend for first-party callersRollout Coordination (deploy/openshift/kustomization.yaml L84-107)

Once this overlay applies, every non-bypassed HTTP/gRPC call (including the control plane's UpdateGateway, which is not in --auth-bypass-methods, and hsctl) must present a token whose aud contains hypershell-frontend. RBAC_SERVICE_ACCOUNTS bypasses the RBAC interceptor but not JWT authentication, so audience enforcement still applies to the control plane. Kind is safe because its realm config already has the mappers, but the OpenShift/external-SSO path is not covered by this diff. The overlay comment block (L63-70) already flags the RBAC breaking change; please extend it to state that the SSO must emit hypershell-frontend in aud for the control-plane and CLI clients before/with this rollout, or those callers will start returning 401 immediately after upgrade.

3. [Minor] Framework bump pulls a large TUI transitive footprint into the API server moduleDependency Hygiene (components/api-server/go.mod L40-125)

The rh-trex-ai bump adds a sizable set of indirect Charmbracelet/TUI dependencies (bubbletea, lipgloss, bubbles, termenv, go-runewidth, etc.) that are unrelated to JWT validation. This is inherited from upstream and not caused by this PR, but it is worth confirming these are intended upstream additions and that none of the new recent transitive modules trip the dependency-age policy (the allowlist only pins rh-trex-ai directly). make check reportedly passes, so this is informational.

Test Diff Scrutiny

No test files are modified in this PR, and no existing assertions were flipped. The behavioral change (audience/issuer now required) is enforced in the upstream rh-trex-ai module and is expressed here as new spec scenarios (oidc-integration.spec.md: "Token Issued for Another Resource Is Rejected") rather than modified in-repo tests. No removed-guarantee pattern detected.

Cross-PR coordination

I reviewed the other open PRs (listed below) for goal, design, ownership, data-model, interface, and change-order conflicts. One material coordination item exists (#206); the rest overlap only at the file level without a design conflict.

Material coordination — #206 [HYPERSHELL-133] hsctl login: This PR and #206 are two halves of the same audience contract and must be reasoned about together.

  • Same design surface / shared assumption: #182 enforces aud=hypershell-frontend on the management API and states that first-party callers (hsctl, control plane) obtain that audience "from dedicated Keycloak mappers." #206 implements exactly that on the CLI side: it adds a new public hypershell-cli Keycloak client whose audience mapper "includes hypershell-frontend in the aud claim so the API server's JWT validator accepts CLI-issued tokens." The two designs agree; there is no incompatibility, but the CLI half of #182's premise is only true once #206 lands.
  • Change order: If #182's audience enforcement reaches an environment whose CLI tokens do not yet carry hypershell-frontend, hsctl calls will fail with 401. The pre-#206 CLI used the password grant on hypershell-frontend (which already carries the audience), so existing CLI usage survives; but the new hypershell-cli client introduced by #206 must ship with its audience mapper before its Auth-Code/Device flows are used against an audience-enforcing API server.
  • Overlapping spec edit: Both PRs edit the same decision table in specs/platform/oidc-integration.spec.md (the "Rationale/Decision" section near the directAccessGrantsEnabled and client-reuse rows). This is a mergeable text overlap, but maintainers should ensure the merged table keeps both #182's "Shared management API resource audience" row and #206's hypershell-cli client rationale consistent.
  • Decision needed: Confirm the intended merge order (or land together) and that the hypershell-cli audience mapper is in place wherever audience enforcement is turned on.

Non-material overlaps (no design conflict):

  • #211 fix(kind): expose gateway metrics and fix control plane connectivity edits deploy/kind/kustomization.yaml, but a different patch block (control-plane env vars + dnsConfig near L91) than #182's API-server JWT patch (near L131-139). Only a potential textual merge, no competing design.
  • #214 / #209 (Dashboard UI) touch components/web-console/bff/src/auth.ts and its tests, but change realm-role/group handling, not the aud check. No interaction with #182's audience enforcement (the BFF still validates aud == OIDC_CLIENT_ID).
  • #194 (feat(control-plane): adopt upstream OpenShell Helm chart) modifies components/control-plane/go.mod, a different module; there is no competing rh-trex-ai bump in the API server go.mod among the open PRs.
  • #216 (fix(console): support OpenShift Route ingress) touches deploy/base/controller-rbac.yaml only; no auth/audience overlap.

Findings Summary (ordered by severity, highest first)

  1. [Minor] --jwt-audience hardcoded while issuer/JWKS are secret-sourced (config asymmetry) — Config vs Code (openshift L107, kind L139)
  2. [Minor] Production rollout requires external SSO to emit aud=hypershell-frontend for control-plane and CLI callers; document alongside the existing RBAC breaking-change note — Rollout Coordination (openshift L63-107)
  3. [Minor] Framework bump adds a large TUI transitive dependency footprint to the API server module — Dependency Hygiene (go.mod L40-125)

Convention Checklist

Convention Result
No secrets in logs or responses Pass
Input validated (issuer/audience enforced) Pass
SecurityContext on pod specs N/A (no pod spec changes)
Image references consistent across manifests N/A
Config separate from code Pass (minor asymmetry noted)
Spec updated to match behavior Pass
OpenAPI client not manually edited N/A
Conventional commit message Pass
Test Diff Scrutiny (no flipped assertions) Pass

value: "--jwt-issuer=$(JWT_ISSUER)"
- op: add
path: /spec/template/spec/containers/0/command/-
value: "--jwt-audience=hypershell-frontend"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] Config asymmetry. --jwk-cert-url and --jwt-issuer are sourced from the hypershell-api-config secret, but the audience is a hardcoded literal here. That's fine if hypershell-frontend is intentionally a fixed platform constant, but it means a deployment that renames its audience must edit this manifest rather than just a secret. Consider sourcing it from the secret for symmetry, or add a note that the resource audience is deliberately fixed.

valueFrom:
secretKeyRef:
name: hypershell-api-config
key: api-service.issuerUrl

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] Rollout coordination. Once this overlay applies, audience enforcement covers every non-bypassed HTTP/gRPC call, including the control plane's UpdateGateway (not in --auth-bypass-methods) and hsctl. RBAC_SERVICE_ACCOUNTS bypasses the RBAC interceptor but not JWT auth, so the control-plane token must carry aud=hypershell-frontend. Kind is safe (its realm config already has the mapper at deploy/base/keycloak/keycloak.yaml L204-219), but on external SSO this is out-of-band. Please extend the breaking-change comment above (L63-70) to state that the SSO must emit hypershell-frontend in aud for the control-plane and CLI clients before/with this rollout, or those callers 401 immediately after upgrade.

github.com/cenkalti/backoff/v5 v5.0.3 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/charmbracelet/bubbles v1.0.0 // indirect
github.com/charmbracelet/bubbletea v1.3.10 // indirect

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] Dependency footprint (informational). The rh-trex-ai bump pulls in a large set of Charmbracelet/TUI indirect deps (bubbletea, lipgloss, bubbles, termenv, etc.) unrelated to JWT validation. Inherited from upstream, not caused by this PR, but worth confirming these are intended and that the new recent transitive modules don't trip the dependency-age policy (the allowlist only pins rh-trex-ai directly).

@jsell-rh

jsell-rh commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator Author

Amber review: comment

Amber review

Status: Complete

Verdict

COMMENT - This change is a well-scoped, security-positive hardening: it bumps rh-trex-ai to the fail-closed issuer/audience validator and wires --jwt-issuer/--jwt-audience into both the Kind and OpenShift overlays, with the spec updated to explain the aud-identifies-resource / azp-identifies-client model. The design is internally consistent (Keycloak KC_HOSTNAME pins iss to the external URL for in-cluster callers, and the frontend + control-plane clients already carry the hypershell-frontend audience mapper), but the production rollout ordering and an unexpected dependency expansion deserve attention before merge.

What I verified

  • Kind auth still works after enforcement. KC_HOSTNAME=https://keycloak.hypershell.localhost pins the iss claim, so the internally-connecting control plane still gets iss=https://keycloak.hypershell.localhost/realms/hypershell, matching --jwt-issuer. Both hypershell-frontend and hypershell-control-plane clients have an oidc-audience-mapper emitting hypershell-frontend, and the control plane's non-bypassed UpdateGateway writes will therefore pass validation.
  • hypershell-provisioner has no audience mapper, but per the spec it only performs Keycloak administration and never calls the management API, so the missing audience is correct, not a regression.
  • Secret key reuse is consistent. api-service.issuerUrl is already consumed by OIDC_ISSUER elsewhere in the OpenShift overlay, so the new JWT_ISSUER reference does not introduce an undefined key. $(JWT_ISSUER) substitution is valid because the env var is declared earlier in the same container.
  • No production code, tests, or generated OpenAPI client were touched; Test Diff Scrutiny found no flipped assertions.

Findings

[Major] Production/OpenShift rollout of required audience+issuer is fail-closed but lacks an operator ordering note.
The OpenShift overlay turns on --jwt-audience=hypershell-frontend and --jwt-issuer=$(JWT_ISSUER) in the same patch that already carries the RBAC "BREAKING CHANGE" note. Enforcement is fail-closed: once applied, every first-party token (web console, hsctl, control plane) presented to the management API must carry aud=hypershell-frontend and the exact iss, or all authenticated HTTP/gRPC requests are rejected immediately after upgrade. Unlike the RBAC change - which has an explicit operator prerequisite ("the SSO MUST define gateway:creator ... coordinate the SSO role mapping with the release") - the audience/issuer prerequisite is only implied by the design-rationale prose. Recommend adding a parallel operator note (in the overlay comment and/or the spec's Production Rollout section) stating that the external SSO must emit aud=hypershell-frontend for all first-party clients and the correct iss before/with this overlay is applied. Confidence: Medium.

[Minor] The dependency bump pulls in a terminal-UI stack unrelated to JWT validation.
The rh-trex-ai bump adds 7 new charmbracelet/* (bubbletea/lipgloss) indirect modules plus supporting deps (atotto/clipboard, rivo/uniseg, muesli/*, mattn/*, etc.) - none were present before. This meaningfully expands the supply-chain surface for what is described as an auth-validation upgrade. Please confirm this transitive pull is intended, and verify the new modules satisfy the dependency-age policy (only rh-trex-ai and otlp are in dependency-age-allowlist.json; if any new transitive module is inside the cooldown window the age gate may fail). Confidence: Medium.

Cross-PR coordination

The management-API audience contract aud=hypershell-frontend that this PR makes required is the same contract that #206 ("hsctl login") supplies for the CLI: #206 adds a new hypershell-cli Keycloak client (absent on main today) whose oidc-audience-mapper emits hypershell-frontend specifically "so the API server's JWT validator accepts CLI-issued tokens." This PR's own spec text already assumes hsctl "receives that audience from dedicated Keycloak mappers," but this PR does not create that client. Both PRs also edit the same authoritative realm (deploy/base/keycloak/keycloak.yaml) and the same specs/platform/oidc-integration.spec.md audience rationale. Maintainers should coordinate merge ordering and reconcile the overlapping realm/spec edits so the hypershell-cli audience mapper is present when (or before) this PR's enforcement is active; otherwise hsctl-issued tokens would be rejected by the management API.

Findings Summary (ordered by severity, highest first)

  1. [Major] Fail-closed audience/issuer enforcement in the OpenShift overlay lacks an explicit operator rollout-ordering note (SSO must emit aud/iss first) - Deployment / Rollout Coordination (deploy/openshift/kustomization.yaml L86-L109)
  2. [Minor] rh-trex-ai bump adds an unrelated charmbracelet TUI dependency stack; confirm intent and dependency-age policy - Dependencies / Supply Chain (components/api-server/go.mod L47-L48)

Convention Checklist

Convention Result
No secrets in logs or responses Pass
Secrets stored as K8s Secret references Pass
Input validated (issuer/audience config) Pass
Image references consistent across manifests Pass
OpenAPI client not manually edited Pass
Conventional commit message Pass
Spec updated to match behavior Pass
Test Diff Scrutiny (no flipped assertions) Pass
Dependency-age allowlist justified Review

@jsell-rh jsell-rh left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

COMMENT - This change is a well-scoped, security-positive hardening: it bumps rh-trex-ai to the fail-closed issuer/audience validator and wires --jwt-issuer/--jwt-audience into both the Kind and OpenShift overlays, with the spec updated to explain the aud-identifies-resource / azp-identifies-client model. The design is internally consistent (Keycloak KC_HOSTNAME pins iss to the external URL for in-cluster callers, and the frontend + control-plane clients already carry the hypershell-frontend audience mapper), but the production rollout ordering and an unexpected dependency expansion deserve attention before merge.

What I verified

  • Kind auth still works after enforcement. KC_HOSTNAME=https://keycloak.hypershell.localhost pins the iss claim, so the internally-connecting control plane still gets iss=https://keycloak.hypershell.localhost/realms/hypershell, matching --jwt-issuer. Both hypershell-frontend and hypershell-control-plane clients have an oidc-audience-mapper emitting hypershell-frontend, and the control plane's non-bypassed UpdateGateway writes will therefore pass validation.
  • hypershell-provisioner has no audience mapper, but per the spec it only performs Keycloak administration and never calls the management API, so the missing audience is correct, not a regression.
  • Secret key reuse is consistent. api-service.issuerUrl is already consumed by OIDC_ISSUER elsewhere in the OpenShift overlay, so the new JWT_ISSUER reference does not introduce an undefined key. $(JWT_ISSUER) substitution is valid because the env var is declared earlier in the same container.
  • No production code, tests, or generated OpenAPI client were touched; Test Diff Scrutiny found no flipped assertions.

Findings

[Major] Production/OpenShift rollout of required audience+issuer is fail-closed but lacks an operator ordering note.
The OpenShift overlay turns on --jwt-audience=hypershell-frontend and --jwt-issuer=$(JWT_ISSUER) in the same patch that already carries the RBAC "BREAKING CHANGE" note. Enforcement is fail-closed: once applied, every first-party token (web console, hsctl, control plane) presented to the management API must carry aud=hypershell-frontend and the exact iss, or all authenticated HTTP/gRPC requests are rejected immediately after upgrade. Unlike the RBAC change - which has an explicit operator prerequisite ("the SSO MUST define gateway:creator ... coordinate the SSO role mapping with the release") - the audience/issuer prerequisite is only implied by the design-rationale prose. Recommend adding a parallel operator note (in the overlay comment and/or the spec's Production Rollout section) stating that the external SSO must emit aud=hypershell-frontend for all first-party clients and the correct iss before/with this overlay is applied. Confidence: Medium.

[Minor] The dependency bump pulls in a terminal-UI stack unrelated to JWT validation.
The rh-trex-ai bump adds 7 new charmbracelet/* (bubbletea/lipgloss) indirect modules plus supporting deps (atotto/clipboard, rivo/uniseg, muesli/*, mattn/*, etc.) - none were present before. This meaningfully expands the supply-chain surface for what is described as an auth-validation upgrade. Please confirm this transitive pull is intended, and verify the new modules satisfy the dependency-age policy (only rh-trex-ai and otlp are in dependency-age-allowlist.json; if any new transitive module is inside the cooldown window the age gate may fail). Confidence: Medium.

Cross-PR coordination

The management-API audience contract aud=hypershell-frontend that this PR makes required is the same contract that #206 ("hsctl login") supplies for the CLI: #206 adds a new hypershell-cli Keycloak client (absent on main today) whose oidc-audience-mapper emits hypershell-frontend specifically "so the API server's JWT validator accepts CLI-issued tokens." This PR's own spec text already assumes hsctl "receives that audience from dedicated Keycloak mappers," but this PR does not create that client. Both PRs also edit the same authoritative realm (deploy/base/keycloak/keycloak.yaml) and the same specs/platform/oidc-integration.spec.md audience rationale. Maintainers should coordinate merge ordering and reconcile the overlapping realm/spec edits so the hypershell-cli audience mapper is present when (or before) this PR's enforcement is active; otherwise hsctl-issued tokens would be rejected by the management API.

Findings Summary (ordered by severity, highest first)

  1. [Major] Fail-closed audience/issuer enforcement in the OpenShift overlay lacks an explicit operator rollout-ordering note (SSO must emit aud/iss first) - Deployment / Rollout Coordination (deploy/openshift/kustomization.yaml L86-L109)
  2. [Minor] rh-trex-ai bump adds an unrelated charmbracelet TUI dependency stack; confirm intent and dependency-age policy - Dependencies / Supply Chain (components/api-server/go.mod L47-L48)

Convention Checklist

Convention Result
No secrets in logs or responses Pass
Secrets stored as K8s Secret references Pass
Input validated (issuer/audience config) Pass
Image references consistent across manifests Pass
OpenAPI client not manually edited Pass
Conventional commit message Pass
Spec updated to match behavior Pass
Test Diff Scrutiny (no flipped assertions) Pass
Dependency-age allowlist justified Review

value: "--jwt-issuer=$(JWT_ISSUER)"
- op: add
path: /spec/template/spec/containers/0/command/-
value: "--jwt-audience=hypershell-frontend"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Major] Add an operator rollout-ordering note for fail-closed audience/issuer enforcement.

This line (with JWT_ISSUER above) turns on required aud=hypershell-frontend + exact iss validation. Enforcement is fail-closed: after this overlay is applied, every first-party token (web console, hsctl, control plane) must carry the audience and matching issuer or all authenticated HTTP/gRPC requests are rejected immediately.

The adjacent RBAC block already documents its SSO prerequisite ("the SSO MUST define gateway:creator ... coordinate the SSO role mapping with the release"), but the audience/issuer prerequisite is only implied by prose elsewhere. Please add a parallel note here (and/or in the spec's Production Rollout section) stating the external SSO must emit aud=hypershell-frontend for all first-party clients and the correct iss before/with this overlay is applied. Confidence: Medium.

github.com/cenkalti/backoff/v5 v5.0.3 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/charmbracelet/bubbles v1.0.0 // indirect
github.com/charmbracelet/bubbletea v1.3.10 // indirect

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Minor] Unexpected dependency expansion from the auth bump.

The rh-trex-ai upgrade transitively adds a full terminal-UI stack (charmbracelet/bubbles, bubbletea, lipgloss, plus atotto/clipboard, rivo/uniseg, muesli/*, mattn/*, etc.) - none present before this PR. That is a notable supply-chain surface increase for a change described as JWT issuer/audience validation.

Please confirm this transitive pull is intended, and verify the new modules satisfy the dependency-age policy: only rh-trex-ai and otlp are listed in dependency-age-allowlist.json, so if any newly added transitive module falls inside the cooldown window the age gate may fail. Confidence: Medium.

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