[] Create helm/values.schema.json with JSON Schema validation including enum, minimum, and if/then dependency rules#289
Conversation
…luding enum, minimum, and if/then dependency rules
![orange-codens[bot]](https://avatars.githubusercontent.com/in/4028936?s=60&v=4){kind=small}
There was a problem hiding this comment.
🍊 Orange Codens レビュー
重大な構造問題が1件あります。 トップレベルに if/then を直書きしつつ allOf も併置している構造は、PRD の設計意図（両ルールを allOf で管理）と不整合であり、将来的な保守性を損ないます（critical）。また allOf 内の外部DB条件に required: ["global"] が欠落しており、global キー省略時に then が誤発動する可能性があります（high）。persistence.repositories.size の Kubernetes quantity パターンも一部不完全です（medium）。全体的に JSON Schema 自体は valid な draft-07 であり Helm は読み込めますが、上記の条件漏れによりバリデーションの抜けが発生します。
🔒 セキュリティ: このPRはHelm install時のバリデーションスキーマを追加するものです。セキュリティ観点での主要な問題は、if/then 条件ブロックにおける required の不備により、ingress.enabled や global.databaseType を省略することでバリデーションチェックがバイパスされる点です（CWE-20）。外部DBモード時の externalDatabase.host 必須チェックも同様のバイパスリスクがあり、意図した安全な設定の強制が機能しない恐れがあります。persistence.size のパターン正規表現も Kubernetes の実際の quantity 仕様と乖離があるため修正を推奨します。
🔴 critical: 1 / 🟠 high: 1 / 🟡 medium: 4 / 🔵 low: 3
その他の指摘 (7 件)
- 🟡 [medium]
helm/values.schema.json:73—persistence.repositories.sizeの pattern が Kubernetes quantity の一部しかカバーしていない (confidence: 0.85,code.schema.pattern_incomplete_quantity) - 🟡 [medium]
helm/values.schema.json:97— ingress.enabled が required でないため if/then 条件が空オブジェクトでバイパス可能 (confidence: 0.85,sec.sast.validation.schema_bypass) - 🟡 [medium]
helm/values.schema.json:117— external DB の if/then で global が required でないため条件バイパス可能 (confidence: 0.82,sec.sast.validation.schema_bypass) - 🟡 [medium]
helm/values.schema.json:32—autoscaling.maxReplicas >= minReplicasの制約が存在しない (confidence: 0.80,code.schema.autoscaling_maxreplicas_no_relation) - 🔵 [low]
helm/values.schema.json:1— 各プロパティにdescriptionフィールドがなく、helm show values/ IDE での補完が機能しない (confidence: 0.75,maintainability.schema.no_description) - 🔵 [low]
helm/values.schema.json:71— persistence.size のパターンが Ki/Mi/Gi/Ti 単位を制限せず不完全 (confidence: 0.70,sec.sast.validation.weak_pattern) - 🔵 [low]
helm/values.schema.json:4— additionalProperties: true により未定義キーが無制限に受け入れられる (confidence: 0.55,sec.sast.validation.open_schema)
head: 3ad9b18 | Orange Codens (P1)
| }, | ||
| "allOf": [ | ||
| { | ||
| "if": { |
There was a problem hiding this comment.
🔴 [critical] トップレベルの if/then と allOf を併用するとセマンティクスが壊れる
JSON Schema draft-07 ではキーワードは独立して評価されます。if/then をトップレベルに直書きしつつ allOf も存在する場合、両者は問題なく共存しますが、トップレベルの if/then は allOf 内に入っていないため、外側の if が false の場合（ingress が存在しないか enabled が true でない場合）に then は無視されます。これ自体はドラフト-07 の仕様通りですが、より重大な問題は allOf と同階層に if/then を置く構造の一貫性のなさです。PRD では「外部DBのif/thenは allOf 経由」と明示されているのに、ingress の if/then だけがトップレベルに分離されており、将来ルールを追加する際に allOf と混在して管理が困難になります。両方を allOf に統一すべきです。
該当箇所:
"if": {
"properties": {
"ingress": {
"properties": {
"enabled": {
"const": true
}
}
}
},
"required": ["ingress"]
},
"then": { ... },
"allOf": [ { "if": ..., "then": ... } ]@@ -93,25 +93,30 @@
- "if": {
- "properties": {
- "ingress": {
- "properties": {
- "enabled": {
- "const": true
- }
- }
- }
- },
- "required": ["ingress"]
- },
- "then": {
- "properties": {
- "ingress": {
- "required": ["host"],
- "properties": {
- "host": {
- "minLength": 1
- }
- }
- }
- }
- },
"allOf": [
+ {
+ "if": {
+ "properties": {
+ "ingress": {
+ "properties": { "enabled": { "const": true } }
+ }
+ },
+ "required": ["ingress"]
+ },
+ "then": {
+ "properties": {
+ "ingress": {
+ "required": ["host"],
+ "properties": { "host": { "minLength": 1 } }
+ }
+ }
+ }
+ },
{code.schema.if_then_allof_conflict | confidence: 0.92
| }, | ||
| "then": { | ||
| "properties": { | ||
| "externalDatabase": { |
There was a problem hiding this comment.
🟠 [high] allOf 内の外部DB if 条件に required: ["global"] がなく、global キーが存在しない場合も条件が通過してしまう
JSON Schema では properties 内のキーワードはそのプロパティが存在する場合にのみ検証されます。global が values.yaml に含まれていない場合、if の properties.global スキーマは「適用対象なし→ true」と評価されてしまい、then の externalDatabase.host required が誤って発動するリスクがあります。
ingress側（トップレベル if）では "required": ["ingress"] が付与されているのに、allOf 内の external DB 側 if には同等の "required": ["global"] が欠落しています。
該当箇所:
"if": {
"properties": {
"global": {
"properties": {
"databaseType": {
"const": "external"
}
}
}
}
},実際には global.databaseType が "external" と明示されていない限り then は発動しないため誤検知の頻度は低いですが、global キーが丸ごと省略された values では if が true になり externalDatabase.host が要求されてしまいます。
@@ -116,7 +116,8 @@
"if": {
"properties": {
"global": {
"properties": {
"databaseType": { "const": "external" }
}
}
- }
+ },
+ "required": ["global"]
},code.schema.if_condition_missing_required | confidence: 0.88
There was a problem hiding this comment.
🍊 Orange Codens レビュー
PRD で要求されたプロパティ定義・enum・minimum・if/then の骨格は概ね揃っていますが、external モード時に externalDatabase オブジェクト自体が欠落してもスキーマを通過する点は PRD 要件を満たさず、マージ前に修正すべきです。ingress 側の条件付きルールは比較的厳密ですが、external 側と enabled の扱いに揃えの余地があります。PRD 検証項目を担保する自動テストも diff には含まれていません。
🔒 セキュリティ: 本 PR は Helm インストール時の values 検証用 JSON Schema (helm/values.schema.json) の新規追加のみで、認証・認可・SQL/コマンド実行・SSRF・暗号処理などのアプリケーション実行パスは diff に含まれていません。additionalProperties: true や host フィールドの minLength のみの制約は設定の柔軟性・入力検証の粒度に関する設計判断ですが、diff 単体から OWASP Top 10 / CWE Top 25 に該当する論理的脆弱性（悪用可能な exploit パス）を確信を持って特定できません。
🔴 critical: 1 / 🟡 medium: 3 / 🔵 low: 1
その他の指摘 (4 件)
- 🟡 [medium]
helm/values.schema.json:1— スキーマ拒否ケースの自動テストが diff に含まれない (confidence: 0.85,maintainability.test.missing_schema_validation) - 🟡 [medium]
helm/values.schema.json:117— external 用 if が global 不在時に常真になり then が弱く適用される (confidence: 0.78,code.logic.if_vacuous_match) - 🟡 [medium]
helm/values.schema.json:90— ingress.enabled=true 未指定でも host 必須が発火する (confidence: 0.72,code.logic.ingress_enabled_not_required) - 🔵 [low]
helm/values.schema.json:90— 条件付きルールの配置が ingress と external で不統一 (confidence: 0.70,maintainability.structure.inconsistent_conditional_placement)
head: 7abdb30 | Orange Codens (P1)
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 [critical] external モード時に externalDatabase オブジェクト欠落を検証できない
PRD は global.databaseType が external のとき externalDatabase.host 欠落を helm install 時に拒否することを要求していますが、then ブロックは properties.externalDatabase 内の required: ["host"] だけを定義しています。JSON Schema draft-07 では、ルートに externalDatabase キーが存在しない場合、そのサブスキーマは適用されず、required も評価されません。したがって global.databaseType: external だけを渡し externalDatabase を丸ごと省略した values は、このスキーマを通過します。
該当箇所:
"then": {
"properties": {
"externalDatabase": {
"required": ["host"],
"properties": {
"host": {
"minLength": 1
}
}
}
}
}ingress 側は if で required: ["ingress"] を付け、then で ingress.required: ["host"] としているため、オブジェクト存在時の必須チェックは機能します。external 側にも同様に then 直下へ required: ["externalDatabase"] が必要です。
@@ -129,6 +129,7 @@
"then": {
+ "required": ["externalDatabase"],
"properties": {
"externalDatabase": {
"required": ["host"],code.logic.conditional_required_gap | confidence: 0.93
1 participant
Summary
Type of Change
Testing
Checklist
Task Goal
Target Repository
Corevice/open-gitAll 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 requires
values.schema.jsonto reject bad installs athelm installtime: wrongdatabaseType, missingexternalDatabase.hostwhen external mode is chosen, missingingress.hostwhen ingress is enabled, and non-integer replica counts. This file must be valid JSON Schema draft-07 understood by Helm.Steps
helm/values.schema.json— root{"$schema": "http://json-schema.org/draft-07/schema#", "type": "object", "additionalProperties": true, "properties": {...}}globalproperty with nesteddatabaseType: {type: "string", enum: ["internal", "external"]}andstorageClass: {type: "string"}replicaCount: {type: "integer", minimum: 1}autoscalingobject withenabled: {type: "boolean"},minReplicas: {type: "integer", minimum: 1},maxReplicas: {type: "integer", minimum: 1},targetCPUUtilizationPercentage: {type: "integer", minimum: 1, maximum: 100}ingressproperty object; add a top-levelif/thenblock:if: {properties: {ingress: {properties: {enabled: {const: true}}}}, required: ["ingress"]}then{properties: {ingress: {required: ["host"], properties: {host: {minLength: 1}}}}}if/thenblock for external database:if: {properties: {global: {properties: {databaseType: {const: "external"}}}}}then{properties: {externalDatabase: {required: ["host"], properties: {host: {minLength: 1}}}}}persistence.repositories.size: {type: "string", pattern: "^[0-9]+[KMGT]i?$"}andpersistence.repositories.retainOnDelete: {type: "boolean"}ssh.serviceType: {type: "string", enum: ["LoadBalancer", "NodePort", "ClusterIP"]}Verification Checklist
helm/values.schema.jsonis valid JSOChanges Made
Creating
helm /values.schema.jsonper the PRD. Checking whether
it already
exists and the repo
layout
.
Creating
helm/values.schema .jsonwith therequired
properties
and conditional
validation blocks
.
Created
helm/values .schema.jsonwithJSON
Schema draft-07
validation for
Helm install
-time
checks
.
**
Schema structure
:**
Root object
with
additional Properties: trueglobal .databaseTypeenum:
internal|
externalre plicaCountintegerwith
minimum: 1aut oscalingwithtyped
fields
and
CPU
percentage
bounds (
1–100)
Top
-level
if/thenrequiring non
-empty
ingress.hostwheningress.enabledistrueSecond
if/then(via
allOf)requiring non-empty
externalDatabase.hostwhen
global.database Typeisexternalpersistence.repositories.sizepatternfor
Kubernetes quantity strings
ssh .serviceTypeenum:
LoadBalancer,NodePort,ClusterIP[P
URPLE-M
ODIFIED-FILES
]
helm/values
.schema.json
[/
PURPLE
-MODIFIED-F
ILES]
Task ID:
019f0d78-fbc2-7333-a937-caab02a15c93Generated by Purple Codens