Skip to content

[] Add helm/templates/secret.yaml (lookup-idempotent), serviceaccount.yaml, rbac.yaml; update deployment.yaml and worker-deployment.yaml with SecurityContext and secret injection#290

Merged
zoetaka38 merged 3 commits into
mainfrom
feature/019f0d78fbc7-019f0d78fbc7
Jun 29, 2026

Conversation

@red-codens

@red-codens red-codens Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Type of Change

  • feat — new feature
  • fix — bug fix
  • chore — tooling / maintenance
  • docs — documentation only
  • refactor — code change that neither fixes a bug nor adds a feature
  • test — adding or updating tests
  • ci — CI/CD configuration

Testing

Checklist

  • Tests pass locally
  • Lint clean (task lint)
  • Commit messages follow Conventional Commits

Task Goal

Target Repository

Corevice/open-git
All work for this task belongs in this repository. Do the implementation inside this repository's directory in the workspace; do not search or edit other repositories unless this task explicitly requires a cross-repo change.

Context

The PRD forbids plain-text secrets in values and requires an existingSecret pattern as first choice with auto-generation as fallback; auto-generated secrets must be idempotent across upgrades (use Helm lookup to avoid regenerating on upgrade). Pods must run as non-root. These changes touch 5 files: 3 new + 2 edits.

Steps

  1. Add helper to helm/templates/_helpers.tpl — define open-git.secretName returning .Values.secrets.existingSecret if non-empty, else {{ include "open-git.fullname" . }}-secrets
  2. Create helm/templates/secret.yaml — wrapped in {{- if not .Values.secrets.existingSecret }}, use {{- $secret := lookup "v1" "Secret" .Release.Namespace (include "open-git.fullname" .) }} to detect existing secret; if $secret is non-nil reuse its data fields; else generate with randAlphaNum 32 | b64enc; keys: db-password, jwt-secret, oauth-client-secret, webhook-secret; add label app.kubernetes.io/component: secrets
  3. Create helm/templates/serviceaccount.yaml — conditional {{- if .Values.serviceAccount.create }}; name from open-git.serviceAccountName helper (returns .Values.serviceAccount.name or open-git.fullname); automountServiceAccountToken: false; merge .Values.serviceAccount.annotations
  4. Create helm/templates/rbac.yaml — conditional on .Values.serviceAccount.create; kind: Role in .Release.Namespace with rules: [{apiGroups: [""], resources: [configmaps, secrets], verbs: [get, list, watch]}]; kind: RoleBinding binding the Role to the ServiceAccount
  5. Edit helm/templates/deployment.yaml — for both backend and frontend containers: add spec.serviceAccountName: {{ include "open-git.serviceAccountName" . }}, add pod-level `securit

Changes Made

Hel
m chart のセ
キュリティ・
シークレット
関連テンプレ
ートを実装
します。まず
既存ファイルを
確認
します


存ファイルを確認
済み。5
ファイルの変更
を実装します

lookup
の Secret
名を修正
し、
deployment
ファイルを更新します

lookup を
Secret 名と
一致させ、 imagePullSecrets
をタ
スク仕
様どおり常
時出力
に修正
します

Hel
m chart

セキュリティ
・シークレ
ット管理の変更
を反映
しました。

変更概要

  1. _helpers.tpl
    open-git .secretName
    existingSecret
    または { fullname}-secrets
    )と open -git.serviceAccountName を追加

secret.yaml
(新規)
existingSecret 未指定
時のみ生成
lookup で既
存 Secret を検
出し、

ければ
4 キ
ーをランダム
生成
3.
serviceaccount.yaml (新
規)— serviceAccount.create
時に
作成、automount ServiceAccountToken: false
4
. rbac .yaml(新
規)— Config
Map/Secret
への get/list/watch
のみ許
可する Role +
RoleBinding
5
. deployment.yaml
backend/frontend
両方
に non
-root security
Context、probe

ス更新
/healthz / /readyz
)、Secret
参照
env、 imagePullSecrets
を追加
6
. worker-deployment .yaml
同様の security
Context と
JWT_SECRET
/WEBHOOK _SECRET env
を追加

[P
URPLE-M
ODIFIED-FILES
]
helm/templates/_
helpers.tpl
helm
/templates/secret.yaml

helm/templates/service
account.yaml
helm
/templates/rbac.yaml

helm/templates/de
ployment.yaml
helm
/templates/worker-deployment
.yaml
[/P
URPLE-M
ODIFIED-FILES
]

Task ID: 019f0d78-fbc7-783c-af3c-0e43d2f64b89
Generated by Purple Codens

…ount.yaml, rbac.yaml; update deployment.yaml and worker-deployment.yaml with SecurityContext and secret injection

@orange-codens orange-codens Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🍊 Orange Codens レビュー

レビュー総評

重大な問題が1件あります: secret.yamllookup 再利用パスで、既存 Secret の data 値を | quote なしで直接展開しているため、値が nil の場合に <nil> という文字列リテラルが Secret に書き込まれます(blocker)。

加えて、3つのDeployment すべてで imagePullSecrets が nil ガードなしで展開される問題(high)があり、.Values.imagePullSecrets 未設定の環境で imagePullSecrets: null が出力されます。

serviceAccountName ヘルパーが serviceAccount.create: false + name 未設定の組み合わせで存在しない SA 名を返す問題、および frontend への JWT_SECRET 注入の設計上の懸念も確認・対応を推奨します。

🔒 セキュリティ: 本 PR では Helm chart へのセキュリティ強化(non-root 実行・Secret lookup・RBAC)が実装されており、方向性は正しいです。最も重要な問題は RBAC Role が全 Secret に対して list/watch を付与している点であり、resourceNames による絞り込みと verbs の最小化が必須です。次に、フロントエンドコンテナへの署名鍵注入は攻撃面を不要に拡大するため設計見直しを推奨します。また readOnlyRootFilesystem: false の明示設定および lookup の quote 漏れも修正することでより堅牢な構成になります。

🛑 blocker: 1 / 🔴 critical: 1 / 🟠 high: 4 / 🟡 medium: 5 / 🔵 low: 5

その他の指摘 (11 件)
  • 🟠 [high] helm/templates/deployment.yaml:139 — frontend Deployment でも imagePullSecrets が nil チェックなしで常時展開される (confidence: 0.90, code.helm.imagepullsecrets_always_rendered_frontend)
  • 🟡 [medium] helm/templates/deployment.yaml:30readOnlyRootFilesystem: false は non-root 強化の目的に反している (confidence: 0.85, maintainability.helm.readonlyrootfilesystem_false)
  • 🟡 [medium] helm/templates/rbac.yaml:3 — Role の metadatanamespace フィールドがない (confidence: 0.80, maintainability.helm.rbac_role_namespace_missing)
  • 🟡 [medium] helm/templates/secret.yaml:13 — lookup で取得した Secret データを quote なしで出力しており YAML インジェクションの可能性 (confidence: 0.80, sec.sast.injection.helm_template_quoting)
  • 🟡 [medium] helm/templates/deployment.yaml:153 — frontend コンテナに JWT_SECRET / OAUTH_CLIENT_SECRET / WEBHOOK_SECRET を注入するのは設計上の懸念がある (confidence: 0.78, maintainability.helm.frontend_jwt_secret_exposure)
  • 🟡 [medium] helm/templates/deployment.yaml:157 — フロントエンドコンテナに JWT_SECRET / OAUTH_CLIENT_SECRET / WEBHOOK_SECRET を注入している (confidence: 0.78, sec.sast.authz.frontend_secret_exposure)
  • 🔵 [low] helm/templates/deployment.yaml:30 — readOnlyRootFilesystem: false でコンテナのファイルシステム書き込みを許可している (confidence: 0.88, sec.sast.hardening.readonly_rootfs_disabled)
  • 🔵 [low] helm/templates/worker-deployment.yaml:30 — worker コンテナでも readOnlyRootFilesystem: false が設定されている (confidence: 0.88, sec.sast.hardening.readonly_rootfs_disabled)
  • 🔵 [low] helm/templates/_helpers.tpl:57open-git.serviceAccountName ヘルパーが serviceAccount.create: false の場合も fullname を返す (confidence: 0.85, maintainability.helm.helpers_serviceaccountname_no_create_guard)
  • 🔵 [low] helm/templates/deployment.yaml:22runAsUser: 1000 / fsGroup: 1000 がハードコードされており values で上書きできない (confidence: 0.82, maintainability.helm.securitycontext_hardcoded_uid)
  • 🔵 [low] helm/templates/secret.yaml:2 — lookup に渡す Secret 名が fullname と異なり既存 Secret を検出できない可能性 (confidence: 0.70, sec.sast.hardening.secret_lookup_key_missing)

head: a78b965 | Orange Codens (P1)

Comment thread helm/templates/secret.yaml Outdated
db-password: {{ index $secret.data "db-password" }}
jwt-secret: {{ index $secret.data "jwt-secret" }}
oauth-client-secret: {{ index $secret.data "oauth-client-secret" }}
webhook-secret: {{ index $secret.data "webhook-secret" }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛑 [blocker] lookup で取得した Secret の data フィールドを quote/nindent せずに直接展開している

既存 Secret が存在する場合、data の値をそのまま展開しています。

該当箇所:

  db-password: {{ index $secret.data "db-password" }}
  jwt-secret: {{ index $secret.data "jwt-secret" }}
  oauth-client-secret: {{ index $secret.data "oauth-client-secret" }}
  webhook-secret: {{ index $secret.data "webhook-secret" }}

lookup が返す data フィールドの値はすでに base64 エンコード済みの文字列なので、そのまま YAML 展開すると値が末尾 \n などを持つ場合に YAML が壊れます。また値が nil(キーが存在しない)の場合、Helm はそれを空文字列ではなく <nil> という文字列として展開し、デプロイ後にアプリが正しい値を得られなくなります。

| default "" | quote を付与するか、| nindent を使って安全にブロックスカラー展開してください:

  db-password: {{ index $secret.data "db-password" | default "" | quote }}
  jwt-secret: {{ index $secret.data "jwt-secret" | default "" | quote }}
  oauth-client-secret: {{ index $secret.data "oauth-client-secret" | default "" | quote }}
  webhook-secret: {{ index $secret.data "webhook-secret" | default "" | quote }}
@@ -13,7 +13,7 @@
-  db-password: {{ index $secret.data "db-password" }}
-  jwt-secret: {{ index $secret.data "jwt-secret" }}
-  oauth-client-secret: {{ index $secret.data "oauth-client-secret" }}
-  webhook-secret: {{ index $secret.data "webhook-secret" }}
+  db-password: {{ index $secret.data "db-password" | default "" | quote }}
+  jwt-secret: {{ index $secret.data "jwt-secret" | default "" | quote }}
+  oauth-client-secret: {{ index $secret.data "oauth-client-secret" | default "" | quote }}
+  webhook-secret: {{ index $secret.data "webhook-secret" | default "" | quote }}

code.helm.secret_lookup_missing_quote | confidence: 0.95

@@ -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

Comment thread helm/templates/rbac.yaml Outdated
rules:
- apiGroups: [""]
resources: ["configmaps", "secrets"]
verbs: ["get", "list", "watch"]

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] RBAC Role が全 Secret への読み取り権限を付与している

CWE-269: Improper Privilege Management

該当箇所:

  - apiGroups: [""]
    resources: ["configmaps", "secrets"]
    verbs: ["get", "list", "watch"]

resources: ["secrets"] に対して verbs: ["list", "watch"] を付与しているため、このネームスペース内の 全 Secret を列挙・ウォッチできます。アプリケーションが lookup パターンで必要とするのは自分自身の Secret への get のみです。list / watch は不要な権限拡大であり、コンテナが侵害された場合に他のサービスの Secret(DB パスワード、API キー等)が漏洩するリスクがあります。

推奨: verbs["get"] のみに絞り、resourceNames で対象 Secret 名を限定してください。

```diff
 rules:
   - apiGroups: [""]
-    resources: ["configmaps", "secrets"]
-    verbs: ["get", "list", "watch"]
+    resources: ["secrets"]
+    resourceNames: ["{{ include \"open-git.secretName\" . }}"]
+    verbs: ["get"]
+  - apiGroups: [""]
+    resources: ["configmaps"]
+    verbs: ["get"]

<sub>`sec.sast.authz.excessive_rbac` | confidence: 0.93</sub>

app.kubernetes.io/component: backend
spec:
serviceAccountName: {{ include "open-git.serviceAccountName" . }}
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

app.kubernetes.io/component: worker
spec:
serviceAccountName: {{ include "open-git.serviceAccountName" . }}
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

…ount.yaml, rbac.yaml; update deployment.yaml and worker-deployment.yaml with SecurityContext and secret injection

@orange-codens orange-codens Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🍊 Orange Codens レビュー

PR 全体の所感

重大な問題が1件あります: secret.yaml の idempotent 保証が helm template(クラスター接続なし)環境では成立しない。lookupnil を返す場合は毎回 randAlphaNum で異なる値が生成されるため、CI パイプラインから helm template | kubectl apply するフローでは upgrade のたびに Secret が上書きされ JWT・OAuth トークンが失効する(PRD 要件違反)。

高優先: RoleBinding への namespace 欠落、RBAC の verbs が PRD 要件 [get,list,watch] と乖離、podSecurityContext ヘルパーで false を意図的に渡しても default true により上書きされる設計バグ(non-root 強制解除不可)が存在する。

probe パス変更(/health/readyz//healthz)はアプリ側対応コミットへの参照がなく、本番でのデプロイ前にアプリ側エンドポイントの確認が必須。

🔒 セキュリティ: 全体として existingSecret パターン・RBAC の最小権限・non-root SecurityContext と readOnlyRootFilesystem の適用など、セキュリティのベストプラクティスに沿った実装になっています。主な懸念点は2点: (1) helm template / dry-run 時に lookup が機能せずランダムシークレットがマニフェストとして出力されるため、CI でキャプチャ・Git push する運用では秘匿措置が必要です。(2) RBAC の configmaps ルールに resourceNames による絞り込みがなく、Namespace 内の全 ConfigMap が読み取れる過剰権限になっています。

🔴 critical: 1 / 🟠 high: 4 / 🟡 medium: 4 / 🔵 low: 4

その他の指摘 (8 件)
  • 🟡 [medium] helm/templates/_helpers.tpl:74open-git.podSecurityContext ヘルパーが YAML フラグメントを直接出力しており、bool 値が文字列になるリスクがある (confidence: 0.92, code.helm.security_context_template_type)
  • 🟡 [medium] helm/templates/rbac.yaml:21 — ConfigMap への get 権限が resourceNames で絞られておらず全 ConfigMap にアクセス可能 (confidence: 0.90, sec.sast.authz.overly_permissive_rbac)
  • 🟡 [medium] helm/templates/deployment.yaml:141 — frontend コンテナに Secret 参照の env が追加されていない(backend との非対称) (confidence: 0.70, code.helm.frontend_no_secret_env)
  • 🟡 [medium] helm/templates/deployment.yaml:113 — backend と frontend が同じ .Values.resources を参照しており、個別チューニングができない (confidence: 0.65, maintainability.helm.resources_shared)
  • 🔵 [low] helm/templates/secret.yaml:12$secret が nil かどうかで同じキー一覧が2回重複定義されている (confidence: 0.82, maintainability.helm.secret_data_duplication)
  • 🔵 [low] helm/templates/deployment.yaml:95 — readinessProbe / livenessProbe のパスが /health から /readyz / /healthz に変更されているがテスト・変更理由の記載がない (confidence: 0.80, maintainability.helm.probe_path_undocumented_change)
  • 🔵 [low] helm/templates/secret.yaml:13 — lookup で取得した既存シークレットの個別キー欠損時に randAlphaNum でその場生成される (confidence: 0.75, sec.sast.crypto.insecure_random)
  • 🔵 [low] helm/templates/rbac.yaml:26 — RoleBinding に namespace が指定されていない (confidence: 0.70, sec.sast.authz.overly_permissive_rbac)

head: dd43f97 | Orange Codens (P1)

@@ -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 ヘルパー経由であり、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

Comment thread helm/templates/rbac.yaml
- {{ 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

Comment thread helm/templates/rbac.yaml
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

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

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

@zoetaka38
zoetaka38 merged commit 9710416 into main Jun 29, 2026
12 of 16 checks passed

@orange-codens orange-codens Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🍊 Orange Codens レビュー

Secret の lookup による idempotent 生成、non-root SecurityContext、existingSecret パターン、ServiceAccount/RBAC の追加は PRD 方向性と概ね整合しています。重大な懸念は Helm のみの PR で backend probe パス変更と frontend probe 新規追加が行われており、アプリ側の /healthz//readyz 対応が同一 changeset に含まれていない点です。RBAC verbs が plan 指定より狭い点、Secret 欠損キー時の部分再生成、テスト不足も確認してください。

🔒 セキュリティ: 本 PR は non-root 実行、readOnlyRootFilesystem、ServiceAccount token 自動マウント無効化、Secret の lookup による冪等生成、env 経由の Secret 注入など、全体的にセキュリティ posture を改善しています。diff 上で確認できる論理的な問題は、RBAC における ConfigMap 権限のスコープ不足(low)程度で、SQL injection・SSRF・認証 bypass・暗号誤用など OWASP/CWE Top 25 に該当する重大な脆弱性は見当たりません。

🟠 high: 2 / 🟡 medium: 3 / 🔵 low: 1 / ⚪ info: 1

その他の指摘 (5 件)
  • 🟡 [medium] helm/templates/rbac.yaml:19 — RBAC verbs が purple plan 指定の get/list/watch と不一致 (confidence: 0.88, maintainability.rbac.plan_deviation)
  • 🟡 [medium] helm/templates/secret.yaml:13 — lookup 済み Secret に欠損キーがあると upgrade 時にそのキーだけ再生成される (confidence: 0.78, code.secret.partial_key_regeneration)
  • 🟡 [medium] helm/templates/secret.yaml:14 — Secret が db-password を生成するが、同一 PR の Deployment env 追加に含まれていない (confidence: 0.62, maintainability.secret.unused_db_password_key)
  • 🔵 [low] helm/templates/rbac.yaml:21 — ConfigMap への get 権限が namespace 内の全リソースに及ぶ (confidence: 0.62, sec.sast.authz.excessive_permissions)
  • [info] helm/templates/secret.yaml:1 — lookup ベースの idempotent Secret 生成に対するテスト追加がない (confidence: 0.85, maintainability.test.missing_helm_template_tests)

head: 53ae440 | Orange Codens (P1)

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

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

@zoetaka38
zoetaka38 deleted the feature/019f0d78fbc7-019f0d78fbc7 branch July 2, 2026 10:50
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