Harden security-sensitive deployment paths#38
Open
tomazb wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a safer, configurable in-cluster Helm install flow (digest-pinned runner image + dedicated RBAC) and hardens Helm repo index fetching for development, while updating Helm chart/deploy assets to support runtime configuration injection and “fail-closed” auth behavior.
Changes:
- Introduces
buildHelmInstallResourcesto generate ServiceAccount/Role/RoleBinding/Job for Helm installs and wires it into the Helm UI tab with runtime-configured runner image/SA. - Adds an SSRF-resistant Helm repo index fetcher for the dev proxy and corresponding tests.
- Updates Helm charts/deploy scripts to inject runtime config (
config.js), remove service-account-token fallback in nginx, and adjust agent dependency compatibility.
Reviewed changes
Copilot reviewed 17 out of 21 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/kubeview/views/create/helmInstall.ts | New builder for Helm install K8s resources + digest pin validation and RBAC/job spec. |
| src/kubeview/views/create/tests/helmInstall.test.ts | Unit tests for digest pin enforcement and resource wiring. |
| src/kubeview/views/create/HelmTab.tsx | Uses runtime config + resource builder; creates SA/RBAC before creating Job. |
| src/kubeview/config/runtime.ts | Reads runtime Helm install config from window.__OPENSHIFTPULSE_CONFIG__. |
| src/dev/helmRepoProxy.ts | New SSRF-protected resolver/fetcher for Helm repo index.yaml. |
| src/dev/tests/helmRepoProxy.test.ts | Tests for hostname resolution validation and URL normalization. |
| rspack.config.ts | Dev endpoint now uses fetchHelmRepoIndex instead of ad-hoc validation/fetch. |
| public/index.html | Loads /config.js at startup. |
| public/config.js | Establishes default global config object. |
| deploy/test-helm.sh | Updates Helm render checks and adds assertions for “fail-closed” token proxy + MCP default. |
| deploy/helm/pulse/values.yaml | Adds openshiftpulse.helmInstall values + MCP disable + compat values. |
| deploy/helm/pulse/Chart.yaml | Updates agent dependency version. |
| deploy/helm/pulse/Chart.lock | Updates dependency lock digest/generated timestamp. |
| deploy/helm/openshiftpulse/values.yaml | Adds helmInstall values with “fail closed” runner image default. |
| deploy/helm/openshiftpulse/templates/nginx-config.yaml | Injects config.js via ConfigMap and removes SA-token fallback; requires forwarded token. |
| deploy/helm/openshiftpulse/templates/deployment.yaml | Mounts config.js from ConfigMap and removes SA-token injection. |
| deploy/deploy.sh | Builds/pushes a dedicated Helm runner image and writes digest-pinned ref into Helm values. |
| Dockerfile.helm-runner | New Helm runner image build with checksum verification and non-root user. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+10
to
+14
| function assertDigestPinnedImage(image: string): void { | ||
| if (!/^[^@]+@sha256:[a-f0-9]{6,}$/i.test(image)) { | ||
| throw new Error('Helm runner image must be digest-pinned'); | ||
| } | ||
| } |
Comment on lines
+104
to
+109
| containers: [{ | ||
| name: 'helm', | ||
| image: input.runnerImage, | ||
| imagePullPolicy: 'IfNotPresent', | ||
| command: ['helm'], | ||
| args: ['install', input.releaseName, input.chartName, '--repo', input.repoUrl, '--namespace', input.namespace, '--wait', '--timeout', '5m'], |
| export function buildHelmInstallResources(input: BuildHelmInstallResourcesInput) { | ||
| assertDigestPinnedImage(input.runnerImage); | ||
|
|
||
| const labels = { app: 'helm-install', chart: input.chartName }; |
Comment on lines
+40
to
+45
| rules: [ | ||
| { | ||
| apiGroups: [''], | ||
| resources: ['configmaps', 'endpoints', 'persistentvolumeclaims', 'pods', 'secrets', 'serviceaccounts', 'services'], | ||
| verbs: ['get', 'list', 'watch', 'create', 'update', 'patch', 'delete'], | ||
| }, |
Comment on lines
+93
to
+97
| spec: { | ||
| backoffLimit: 0, | ||
| template: { | ||
| spec: { | ||
| restartPolicy: 'Never', |
Comment on lines
+16
to
+21
| export function getHelmInstallRuntimeConfig(): Required<HelmInstallRuntimeConfig> { | ||
| return { | ||
| runnerImage: window.__OPENSHIFTPULSE_CONFIG__?.helmInstall?.runnerImage ?? '', | ||
| serviceAccountName: window.__OPENSHIFTPULSE_CONFIG__?.helmInstall?.serviceAccountName ?? 'openshiftpulse-helm-installer', | ||
| }; | ||
| } |
Comment on lines
+9
to
+15
| config.js: | | ||
| window.__OPENSHIFTPULSE_CONFIG__ = { | ||
| helmInstall: { | ||
| runnerImage: {{ .Values.helmInstall.runnerImage | quote }}, | ||
| serviceAccountName: {{ .Values.helmInstall.serviceAccountName | quote }} | ||
| } | ||
| }; |
Comment on lines
+54
to
+57
| if ($http_x_forwarded_access_token = '') { | ||
| return 401 '{"error":"X-Forwarded-Access-Token header is required"}'; | ||
| } | ||
| proxy_set_header Authorization "Bearer $k8s_token"; | ||
| proxy_set_header Authorization "Bearer $http_x_forwarded_access_token"; |
| @@ -0,0 +1,17 @@ | |||
| FROM registry.access.redhat.com/ubi9/ubi-minimal:latest | |||
Comment on lines
+6
to
+9
| <meta name="description" content="OpenShift Pulse — Next-generation OpenShift Console" /> | ||
| <title>OpenShift Pulse</title> | ||
| <script src="/config.js"></script> | ||
| <link rel="icon" type="image/svg+xml" href="data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 32 32'%3E%3Cdefs%3E%3ClinearGradient id='bg' x1='0' y1='0' x2='1' y2='1'%3E%3Cstop offset='0%25' stop-color='%232563eb'/%3E%3Cstop offset='100%25' stop-color='%237c3aed'/%3E%3C/linearGradient%3E%3ClinearGradient id='ln' x1='0' y1='0' x2='1' y2='0'%3E%3Cstop offset='0%25' stop-color='%2360a5fa'/%3E%3Cstop offset='50%25' stop-color='%23ffffff'/%3E%3Cstop offset='100%25' stop-color='%23a78bfa'/%3E%3C/linearGradient%3E%3C/defs%3E%3Crect width='32' height='32' rx='7' fill='url(%23bg)'/%3E%3Cpolyline points='4,16.5 9,16.5 11,16.5 13,12 15,21 17,7 19,25 21,13 23,16.5 25,16.5 28,16.5' fill='none' stroke='url(%23ln)' stroke-width='1.8' stroke-linecap='round' stroke-linejoin='round'/%3E%3Ccircle cx='17' cy='7' r='1.2' fill='white' opacity='0.5'/%3E%3C/svg%3E" /> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
The reviewed deployment paths could otherwise grant broad service-account access, expose an unauthenticated MCP control surface, run mutable third-party Helm images, or let dev proxy requests reach private services through DNS rebinding.
Validation