Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions helm/templates/_helpers.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,47 @@ Chart label
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | trimSuffix "-" }}
{{- end }}

{{/*
Secret name — existingSecret if set, otherwise auto-generated name.
*/}}
{{- define "open-git.secretName" -}}
{{- if .Values.secrets.existingSecret }}
{{- .Values.secrets.existingSecret }}
{{- else }}
{{- printf "%s-secrets" (include "open-git.fullname" .) }}
{{- end }}
{{- end }}

{{/*
ServiceAccount name
*/}}
{{- define "open-git.serviceAccountName" -}}
{{- if .Values.serviceAccount.name }}
{{- .Values.serviceAccount.name }}
{{- else if .Values.serviceAccount.create }}
{{- include "open-git.fullname" . }}
{{- else }}
{{- "default" }}
{{- end }}
{{- end }}

{{/*
Pod-level security context
*/}}
{{- define "open-git.podSecurityContext" -}}
runAsNonRoot: {{ .Values.securityContext.runAsNonRoot | default true }}
runAsUser: {{ .Values.securityContext.runAsUser | default 1000 }}
fsGroup: {{ .Values.securityContext.fsGroup | default (.Values.securityContext.runAsUser | default 1000) }}
{{- end }}

{{/*
Container-level security context
*/}}
{{- define "open-git.containerSecurityContext" -}}
allowPrivilegeEscalation: false
readOnlyRootFilesystem: true
{{- end }}

{{/*
Validated Git repository mount path from values.
*/}}
Expand Down
75 changes: 66 additions & 9 deletions helm/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,26 @@ spec:
{{- include "open-git.selectorLabels" . | nindent 8 }}
app.kubernetes.io/component: backend
spec:
serviceAccountName: {{ include "open-git.serviceAccountName" . }}
{{- with .Values.imagePullSecrets }}
imagePullSecrets:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 [high] imagePullSecrets が nil チェックなしで常時展開されるため、空リストが YAML に出力される

該当箇所:

      imagePullSecrets:
        {{- toYaml .Values.imagePullSecrets | nindent 8 }}

.Values.imagePullSecretsnil または空リストの場合、toYaml nilnull\n を出力し、結果として imagePullSecrets: null という不正な YAML フィールドになります。Kubernetes はこれを空リストとして解釈することもありますが、バリデーション次第では拒否される場合があります。

既存 Helm チャートの慣習に従い、{{- with .Values.imagePullSecrets }} でガードしてください:

      {{- with .Values.imagePullSecrets }}
      imagePullSecrets:
        {{- toYaml . | nindent 8 }}
      {{- end }}

これは worker-deployment.yaml の同箇所(+20-+21行目)も同様です。

@@ -20,3 +20,5 @@
-      imagePullSecrets:
-        {{- toYaml .Values.imagePullSecrets | nindent 8 }}
+      {{- with .Values.imagePullSecrets }}
+      imagePullSecrets:
+        {{- toYaml . | nindent 8 }}
+      {{- end }}

code.helm.imagepullsecrets_always_rendered | confidence: 0.90

{{- toYaml . | nindent 8 }}
{{- end }}
securityContext:
{{- include "open-git.podSecurityContext" . | nindent 8 }}
containers:
- name: backend
image: "{{ .Values.image.backend.repository }}:{{ .Values.image.backend.tag }}"
imagePullPolicy: {{ .Values.image.backend.pullPolicy }}
securityContext:
{{- include "open-git.containerSecurityContext" . | nindent 12 }}
volumeMounts:
- name: tmp
mountPath: /tmp
{{- if (.Values.persistence.repositories.enabled | default true) }}
- name: repositories
mountPath: {{ include "open-git.repositoriesMountPath" . | quote }}
{{- end }}
ports:
- name: http
containerPort: 8080
Expand Down Expand Up @@ -66,33 +82,45 @@ spec:
- name: MINIO_BUCKET
value: {{ .Values.minio.bucket | quote }}
{{- end }}
- name: JWT_SECRET
valueFrom:
secretKeyRef:
name: {{ include "open-git.secretName" . }}
key: jwt-secret
- name: OAUTH_CLIENT_SECRET
valueFrom:
secretKeyRef:
name: {{ include "open-git.secretName" . }}
key: oauth-client-secret
- name: WEBHOOK_SECRET
valueFrom:
secretKeyRef:
name: {{ include "open-git.secretName" . }}
key: webhook-secret
readinessProbe:
httpGet:
path: /health
path: /readyz
port: 8080
initialDelaySeconds: 5
periodSeconds: 10
failureThreshold: 3
livenessProbe:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 [high] backend の probe パスが /health から /healthz・/readyz に変更されている

本 PR の変更ファイルは Helm テンプレートのみですが、backend の probe パスが差し替えられています。/health が削除され、readiness は /readyz、liveness は /healthz になっています。アプリケーション側が同一 PR でこれらのエンドポイントを提供していない場合、Pod が Not Ready になるか liveness 失敗で再起動ループになります。

該当箇所:

           readinessProbe:
             httpGet:
-              path: /health
+              path: /readyz
               port: 8080
...
           livenessProbe:
             httpGet:
-              path: /health
+              path: /healthz
               port: 8080
@@ -100,7 +100,7 @@
           readinessProbe:
             httpGet:
-              path: /readyz
+              path: /health
               port: 8080
           livenessProbe:
             httpGet:
-              path: /healthz
+              path: /health
               port: 8080

code.health_probe.missing_app_support | confidence: 0.82

httpGet:
path: /health
path: /healthz
port: 8080
initialDelaySeconds: 15
periodSeconds: 20
failureThreshold: 3
{{- if (.Values.persistence.repositories.enabled | default true) }}
volumeMounts:
- name: repositories
mountPath: {{ include "open-git.repositoriesMountPath" . | quote }}
{{- end }}
resources:
{{- toYaml .Values.resources | nindent 12 }}
{{- if (.Values.persistence.repositories.enabled | default true) }}
volumes:
- name: tmp
emptyDir: {}
{{- if (.Values.persistence.repositories.enabled | default true) }}
- name: repositories
persistentVolumeClaim:
claimName: {{ include "open-git.fullname" . }}-repositories
{{- end }}
{{- end }}
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
Expand Down Expand Up @@ -125,13 +153,42 @@ spec:
{{- include "open-git.selectorLabels" . | nindent 8 }}
app.kubernetes.io/component: frontend
spec:
serviceAccountName: {{ include "open-git.serviceAccountName" . }}
{{- with .Values.imagePullSecrets }}
imagePullSecrets:
{{- toYaml . | nindent 8 }}
{{- end }}
securityContext:
{{- include "open-git.podSecurityContext" . | nindent 8 }}
containers:
- name: frontend
image: "{{ .Values.image.frontend.repository }}:{{ .Values.image.frontend.tag }}"
imagePullPolicy: {{ .Values.image.frontend.pullPolicy }}
securityContext:
{{- include "open-git.containerSecurityContext" . | nindent 12 }}
volumeMounts:
- name: tmp
mountPath: /tmp
ports:
- name: http
containerPort: 3000
protocol: TCP
readinessProbe:
httpGet:
path: /readyz
port: 3000
initialDelaySeconds: 5
periodSeconds: 10
failureThreshold: 3
livenessProbe:
httpGet:
path: /healthz

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 [high] frontend に新規 probe が追加されているが、同一 PR にアプリ変更がない

frontend Deployment には従来 probe がなく、本 diff で readiness/liveness が新規追加されています。ポート 3000 の /readyz/healthz を前提としていますが、diff 上は Helm テンプレートのみの変更です。frontend がこれらの HTTP エンドポイントを実装していない場合、Rollout 後に Pod が Ready にならない可能性が高いです。

該当箇所:

+          readinessProbe:
+            httpGet:
+              path: /readyz
+              port: 3000
+            initialDelaySeconds: 5
+            periodSeconds: 10
+            failureThreshold: 3
+          livenessProbe:
+            httpGet:
+              path: /healthz
+              port: 3000

code.health_probe.frontend_added | confidence: 0.80

port: 3000
initialDelaySeconds: 15
periodSeconds: 20
failureThreshold: 3
resources:
{{- toYaml .Values.resources | nindent 12 }}
volumes:
- name: tmp
emptyDir: {}
39 changes: 39 additions & 0 deletions helm/templates/rbac.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
{{- if .Values.serviceAccount.create }}
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: {{ include "open-git.fullname" . }}
namespace: {{ .Release.Namespace }}
labels:
{{- include "open-git.labels" . | nindent 4 }}
rules:
- apiGroups: [""]
resources: ["secrets"]
resourceNames:
- {{ include "open-git.secretName" . | quote }}
{{- if eq .Values.db.type "postgres" }}
- {{ printf "%s-db" (include "open-git.fullname" .) | quote }}
{{- end }}
{{- if .Values.minio.enabled }}
- {{ printf "%s-minio" (include "open-git.fullname" .) | quote }}
{{- end }}
verbs: ["get"]
- apiGroups: [""]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 [high] PRD 要件の [get, list, watch] に対し、Role の secrets リソースに listwatch が欠落している

該当箇所:

    resources: ["secrets"]
    resourceNames:
      - {{ include "open-git.secretName" . | quote }}
      ...
    verbs: ["get"]

タスク仕様では verbs: [get, list, watch] と明記されているが、実装は verbs: ["get"] のみ。configmaps 側も同様:

  - apiGroups: [""]
    resources: ["configmaps"]
    verbs: ["get"]

list / watch が必要かどうかはアプリケーション側の要件次第だが、PRD との乖離がある。また resourceNames 指定がある場合、Kubernetes では listwatchresourceNames フィルタが効かないため、list/watch を付与するなら resourceNames を外す必要があり、設計上のトレードオフを明示すべき。現状では get のみで動くアプリなら問題ないが、PRD との不一致は要確認。

@@ -16,1 +16,1 @@
-    verbs: ["get"]
+    verbs: ["get", "list", "watch"]

code.helm.rbac_role_verbs_mismatch | confidence: 0.88

resources: ["configmaps"]
verbs: ["get"]
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: {{ include "open-git.fullname" . }}
labels:
{{- include "open-git.labels" . | nindent 4 }}
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: {{ include "open-git.fullname" . }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 [high] RoleBindingmetadatanamespace が欠落している

該当箇所:

kind: RoleBinding
metadata:
  name: {{ include "open-git.fullname" . }}
  labels:
    {{- include "open-git.labels" . | nindent 4 }}

Role には namespace: {{ .Release.Namespace }} が明示されているが、RoleBinding には namespace フィールドがない。Helm はデフォルトで --namespace フラグのネームスペースにリソースを作成するため通常は動作するが、helm template --namespace を使わずに kubectl apply した場合や default namespace が意図しない場合に不整合が生じる。Role と同様に namespace: {{ .Release.Namespace }} を明示すべき。

@@ -25,4 +25,5 @@
 metadata:
   name: {{ include "open-git.fullname" . }}
+  namespace: {{ .Release.Namespace }}
   labels:
     {{- include "open-git.labels" . | nindent 4 }}

code.helm.rbac_rolebinding_missing_namespace | confidence: 0.85

subjects:
- kind: ServiceAccount
name: {{ include "open-git.serviceAccountName" . }}
namespace: {{ .Release.Namespace }}
{{- end }}
23 changes: 23 additions & 0 deletions helm/templates/secret.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{{- if not .Values.secrets.existingSecret }}
{{- $secret := lookup "v1" "Secret" .Release.Namespace (include "open-git.secretName" .) }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 [critical] lookup の Secret 名に open-git.secretName を使うと {fullname}-secrets になり、実際に作成される名前と一致しない可能性がある

該当箇所:

{{- $secret := lookup "v1" "Secret" .Release.Namespace (include "open-git.secretName" .) }}

open-git.secretName ヘルパーは existingSecret が空の場合 {fullname}-secrets を返します。ただし metadata.name も同じヘルパーで設定(5行目)されているため、初回インストール時は一致します。しかしアップグレード前後で existingSecret の設定が変わった場合、lookup 対象名と実際の Secret 名がずれるリスクがあります。

設計意図は問題ないものの、_helpers.tpl で定義している open-git.secretName ではなく、明示的に printf "%s-secrets" (include "open-git.fullname" .) を lookup で使うことで、existingSecret が後から設定された場合の誤 lookup を防げます。

code.helm.secret_lookup_wrong_name | confidence: 0.72

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 [critical] lookup で参照する Secret 名が open-git.secretName ヘルパー経由であり、existingSecret が空の場合は {fullname}-secrets を返すが、ガード条件と矛盾しない点は正しい。ただし $secret が nil でも randAlphaNum が毎回異なる値を生成するため idempotent 保証が壊れる

該当箇所:

{{- $secret := lookup "v1" "Secret" .Release.Namespace (include "open-git.secretName" .) }}

lookup が Secret を見つけた場合は既存の data フィールドを再利用するが、$secretnil(初回インストール時)のブランチでは:

  {{- else }}
  db-password: {{ randAlphaNum 32 | b64enc | quote }}
  jwt-secret: {{ randAlphaNum 32 | b64enc | quote }}
  oauth-client-secret: {{ randAlphaNum 32 | b64enc | quote }}
  webhook-secret: {{ randAlphaNum 32 | b64enc | quote }}
  {{- end }}

helm template--dry-run を複数回呼ぶたびに毎回異なる値が生成される。より重大なのは、helm upgrade 時に lookup がクラスター内の Secret を取得できるため通常は idempotent だが、CI での helm template (クラスター接続なし) では lookup は常に nil を返し、apply するたびに Secret が上書きされてトークンが失効する。PRD 要件「auto-generated secrets must be idempotent across upgrades」を完全には満たせない。

推奨修正: $secretnil の場合のキーにも lookup と同等の保護を与えるか、または Helm の secretHelper パターンとして randAlphaNum の結果を sha256sum や固定シードで deterministic にすることは不可能なため、helm upgrade --install の運用フローを明記し、helm template での適用を禁止するドキュメントを追加する。あるいは lookup 結果のみに依存し nil 時は空文字を出力してエラーにする設計を検討する。

code.helm.lookup_key_mismatch | confidence: 0.90

apiVersion: v1
kind: Secret
metadata:
name: {{ include "open-git.secretName" . }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 [high] lookup で参照する名前と作成する Secret の名前が一致しているが、existingSecret が設定されていない場合に限り {fullname}-secrets になる点の確認

該当箇所:

{{- $secret := lookup "v1" "Secret" .Release.Namespace (include "open-git.secretName" .) }}
...
  name: {{ include "open-git.secretName" . }}

open-git.secretNameexistingSecret が空の場合 {fullname}-secrets を返すが、このブロック自体が {{- if not .Values.secrets.existingSecret }} で囲まれているため lookup に渡る名前と実際に作成する名前は一致する。設計的には問題ないが、_helpers.tplopen-git.secretName ヘルパー(下記)がフォールバックとして {fullname}-secrets を返すのに対し、以前のタスク仕様では {{ include "open-git.fullname" . }}-secrets と記述されており整合する。

ただし、lookup の第4引数に渡る名前は open-git.secretName 経由であるため、もし将来 existingSecret を後から設定すると lookup に渡る名前も変わり、既存 Secret が孤立したまま Helm 管理外になる。これは運用リスクとして文書化が必要。

code.helm.secret_lookup_name_inconsistency | confidence: 0.75

labels:
{{- include "open-git.labels" . | nindent 4 }}
app.kubernetes.io/component: secrets
type: Opaque
data:
{{- if $secret }}
db-password: {{ index $secret.data "db-password" | default (randAlphaNum 32 | b64enc) | quote }}
jwt-secret: {{ index $secret.data "jwt-secret" | default (randAlphaNum 32 | b64enc) | quote }}
oauth-client-secret: {{ index $secret.data "oauth-client-secret" | default (randAlphaNum 32 | b64enc) | quote }}
webhook-secret: {{ index $secret.data "webhook-secret" | default (randAlphaNum 32 | b64enc) | quote }}
{{- else }}
db-password: {{ randAlphaNum 32 | b64enc | quote }}
jwt-secret: {{ randAlphaNum 32 | b64enc | quote }}
oauth-client-secret: {{ randAlphaNum 32 | b64enc | quote }}
webhook-secret: {{ randAlphaNum 32 | b64enc | quote }}
{{- end }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 [high] helm/template dry-run・CI 環境では lookup が空を返すためシークレットが毎回再生成される

CWE-330: Use of Insufficiently Random Values / CWE-344: Use of Invariant Value in Dynamically Changing Context

lookup は Helm dry-run (--dry-run) や CI の helm template では常に空オブジェクトを返すため、$secret が nil 扱いとなり、以下のコードパスが毎回実行されます。

  {{- else }}
  db-password: {{ randAlphaNum 32 | b64enc | quote }}
  jwt-secret: {{ randAlphaNum 32 | b64enc | quote }}
  oauth-client-secret: {{ randAlphaNum 32 | b64enc | quote }}
  webhook-secret: {{ randAlphaNum 32 | b64enc | quote }}
  {{- end }}

これにより、CI パイプラインがマニフェストをキャプチャして GitOps リポジトリへ push する運用の場合、ランダム生成シークレットが平文で Git に記録されるリスクがあります。また実際のクラスタへの apply 時も lookup の返り値が構造体として扱われるか nil として扱われるかは Helm バージョン依存であり、テンプレート生成フローが異なる環境で異なるシークレット値を出力する設計上の問題があります。

推奨: dry-run / helm template 時に生成されたシークレット値を Git やログに保存しないよう CI プロセスを設計する、または Sealed Secrets / External Secrets Operator などの外部管理方式を第一選択とすることを values のコメントで明示する。

# values.yaml に以下コメントを追加
# secrets:
#   existingSecret: ""  # 本番では必ず existingSecret を指定してください。
#                         # helm template / --dry-run 時は lookup が機能せず
#                         # ランダム値が出力されます。CI でマニフェストを
#                         # キャプチャする場合は出力を秘匿してください。

sec.sast.crypto.insecure_random | confidence: 0.85

{{- end }}
13 changes: 13 additions & 0 deletions helm/templates/serviceaccount.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{{- if .Values.serviceAccount.create }}
apiVersion: v1
kind: ServiceAccount
metadata:
name: {{ include "open-git.serviceAccountName" . }}
labels:
{{- include "open-git.labels" . | nindent 4 }}
{{- with .Values.serviceAccount.annotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
automountServiceAccountToken: false
{{- end }}
25 changes: 25 additions & 0 deletions helm/templates/worker-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,22 @@ spec:
{{- include "open-git.selectorLabels" . | nindent 8 }}
app.kubernetes.io/component: worker
spec:
serviceAccountName: {{ include "open-git.serviceAccountName" . }}
{{- with .Values.imagePullSecrets }}
imagePullSecrets:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 [high] worker-deployment.yaml でも imagePullSecrets が nil チェックなしで常時展開される

該当箇所:

      imagePullSecrets:
        {{- toYaml .Values.imagePullSecrets | nindent 8 }}

deployment.yaml と同じ問題です。.Values.imagePullSecrets が未設定の場合 imagePullSecrets: null が出力されます。{{- with .Values.imagePullSecrets }} でガードしてください。

@@ -20,3 +20,5 @@
-      imagePullSecrets:
-        {{- toYaml .Values.imagePullSecrets | nindent 8 }}
+      {{- with .Values.imagePullSecrets }}
+      imagePullSecrets:
+        {{- toYaml . | nindent 8 }}
+      {{- end }}

code.helm.imagepullsecrets_always_rendered_worker | confidence: 0.90

{{- toYaml . | nindent 8 }}
{{- end }}
securityContext:
{{- include "open-git.podSecurityContext" . | nindent 8 }}
containers:
- name: worker
image: "{{ .Values.image.backend.repository }}:{{ .Values.image.backend.tag }}"
imagePullPolicy: {{ .Values.image.backend.pullPolicy }}
securityContext:
{{- include "open-git.containerSecurityContext" . | nindent 12 }}
volumeMounts:
- name: tmp
mountPath: /tmp
args:
- "--worker-only"
env:
Expand Down Expand Up @@ -59,8 +71,21 @@ spec:
- name: MINIO_BUCKET
value: {{ .Values.minio.bucket | quote }}
{{- end }}
- name: JWT_SECRET
valueFrom:
secretKeyRef:
name: {{ include "open-git.secretName" . }}
key: jwt-secret
- name: WEBHOOK_SECRET
valueFrom:
secretKeyRef:
name: {{ include "open-git.secretName" . }}
key: webhook-secret
resources:
{{- toYaml .Values.resources | nindent 12 }}
volumes:
- name: tmp
emptyDir: {}
{{- with .Values.nodeSelector }}
nodeSelector:
{{- toYaml . | nindent 8 }}
Expand Down
Loading