codeant-ai-for-open-source[bot] commented on code in PR #42753:
URL: https://github.com/apache/superset/pull/42753#discussion_r3710877447


##########
helm/superset/templates/_helpers.tpl:
##########
@@ -208,8 +208,10 @@ Helper to safely read 
.Values.supersetNode.connections.<key> without erroring wh
 
 {{- define "superset.config" }}
 {{- /* SECURITY: Validate admin password is set if admin creation is enabled 
*/}}
-{{- if and .Values.init.createAdmin (or (not .Values.init.adminUser.password) 
(eq .Values.init.adminUser.password "")) }}
-{{- fail "SECURITY ERROR: init.createAdmin is true but init.adminUser.password 
is empty. You must set a secure password using --set 
init.adminUser.password='your-password' or via external secret." }}
+{{- $adminPasswordSet := and .Values.init.adminUser.password (not (eq 
.Values.init.adminUser.password "")) }}
+{{- $adminSecretSet := and .Values.init.adminUser.existingSecret (not (eq 
.Values.init.adminUser.existingSecret "")) }}
+{{- if and .Values.init.createAdmin (not (or $adminPasswordSet 
$adminSecretSet)) }}
+{{- fail "SECURITY ERROR: init.createAdmin is true but neither 
init.adminUser.password nor init.adminUser.existingSecret is set. You must set 
a secure password using --set init.adminUser.password='your-password' or 
provide an existing Kubernetes secret via init.adminUser.existingSecret." }}
 {{- end }}

Review Comment:
   **Suggestion:** The validation runs whenever `superset.config` is included, 
including deployments and the config Secret, but it does not check 
`init.enabled`. Consequently, configurations that disable the init Job while 
leaving `createAdmin` enabled and clearing the unused password will fail Helm 
rendering even though no admin creation will run. Gate this validation on 
`init.enabled` as well. [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Disabled init deployments fail Helm rendering unnecessarily.
   - ⚠️ Config and deployment manifests cannot be generated.
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=21dc51d8e8eb4ed2ae36b5d28fc6f56e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=21dc51d8e8eb4ed2ae36b5d28fc6f56e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** helm/superset/templates/_helpers.tpl
   **Line:** 213:215
   **Comment:**
        *Api Mismatch: The validation runs whenever `superset.config` is 
included, including deployments and the config Secret, but it does not check 
`init.enabled`. Consequently, configurations that disable the init Job while 
leaving `createAdmin` enabled and clearing the unused password will fail Helm 
rendering even though no admin creation will run. Gate this validation on 
`init.enabled` as well.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42753&comment_hash=ff5ba8e045c402721192866d09afaefbeb602f43af24d4412f90b1ab9c48f673&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42753&comment_hash=ff5ba8e045c402721192866d09afaefbeb602f43af24d4412f90b1ab9c48f673&reaction=dislike'>👎</a>



##########
helm/superset/templates/_helpers.tpl:
##########
@@ -700,15 +703,28 @@ superset init
 echo "Init job: Creating admin user and loading initial data..."
 {{- if .Values.init.createAdmin }}
 echo "Creating admin user (if not present)..."
-if superset fab list-users 2>/dev/null | grep -qF {{ printf "username:%s" 
.Values.init.adminUser.username | squote }}; then
+{{- if $adminSecretSet }}
+ADMIN_USERNAME="$SUPERSET_ADMIN_USERNAME"
+ADMIN_FIRSTNAME="$SUPERSET_ADMIN_FIRSTNAME"
+ADMIN_LASTNAME="$SUPERSET_ADMIN_LASTNAME"
+ADMIN_EMAIL="$SUPERSET_ADMIN_EMAIL"
+ADMIN_PASSWORD="$SUPERSET_ADMIN_PASSWORD"
+{{- else }}
+ADMIN_USERNAME={{ .Values.init.adminUser.username | quote }}
+ADMIN_FIRSTNAME={{ .Values.init.adminUser.firstname | quote }}
+ADMIN_LASTNAME={{ .Values.init.adminUser.lastname | quote }}
+ADMIN_EMAIL={{ .Values.init.adminUser.email | quote }}
+ADMIN_PASSWORD={{ .Values.init.adminUser.password | quote }}

Review Comment:
   **Suggestion:** The literal values are rendered inside shell double quotes 
using Helm's `quote`, which is not shell escaping. Values containing `$` or 
backticks can be expanded or command-substituted when the init script runs, and 
embedded shell-sensitive content can alter or break the assignments. Render 
these values with shell-safe single-quote escaping or pass them through 
environment variables instead. [security]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ❌ Malicious credential values execute commands in init containers.
   - ⚠️ Init pod service-account and mounted-secret access may be exposed.
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=29e64cb660d74372895e99eb6b2e6b22&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=29e64cb660d74372895e99eb6b2e6b22&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** helm/superset/templates/_helpers.tpl
   **Line:** 713:717
   **Comment:**
        *Security: The literal values are rendered inside shell double quotes 
using Helm's `quote`, which is not shell escaping. Values containing `$` or 
backticks can be expanded or command-substituted when the init script runs, and 
embedded shell-sensitive content can alter or break the assignments. Render 
these values with shell-safe single-quote escaping or pass them through 
environment variables instead.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42753&comment_hash=779274c8cc233961740922950ddfd4651b1d0de7df623c56d1267401358e082b&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42753&comment_hash=779274c8cc233961740922950ddfd4651b1d0de7df623c56d1267401358e082b&reaction=dislike'>👎</a>



##########
helm/superset/templates/_helpers.tpl:
##########
@@ -700,15 +703,28 @@ superset init
 echo "Init job: Creating admin user and loading initial data..."
 {{- if .Values.init.createAdmin }}
 echo "Creating admin user (if not present)..."
-if superset fab list-users 2>/dev/null | grep -qF {{ printf "username:%s" 
.Values.init.adminUser.username | squote }}; then
+{{- if $adminSecretSet }}
+ADMIN_USERNAME="$SUPERSET_ADMIN_USERNAME"
+ADMIN_FIRSTNAME="$SUPERSET_ADMIN_FIRSTNAME"
+ADMIN_LASTNAME="$SUPERSET_ADMIN_LASTNAME"
+ADMIN_EMAIL="$SUPERSET_ADMIN_EMAIL"
+ADMIN_PASSWORD="$SUPERSET_ADMIN_PASSWORD"
+{{- else }}
+ADMIN_USERNAME={{ .Values.init.adminUser.username | quote }}
+ADMIN_FIRSTNAME={{ .Values.init.adminUser.firstname | quote }}
+ADMIN_LASTNAME={{ .Values.init.adminUser.lastname | quote }}
+ADMIN_EMAIL={{ .Values.init.adminUser.email | quote }}
+ADMIN_PASSWORD={{ .Values.init.adminUser.password | quote }}
+{{- end }}
+if superset fab list-users 2>/dev/null | grep -qF 
'username:'"${ADMIN_USERNAME}"; then
   echo "Admin user already exists, skipping."
 else
   superset fab create-admin \
-      --username {{ .Values.init.adminUser.username | squote }} \
-      --firstname {{ .Values.init.adminUser.firstname | squote }} \
-      --lastname {{ .Values.init.adminUser.lastname | squote }} \
-      --email {{ .Values.init.adminUser.email | squote }} \
-      --password {{ .Values.init.adminUser.password | squote }}
+      --username "${ADMIN_USERNAME}" \
+      --firstname "${ADMIN_FIRSTNAME}" \
+      --lastname "${ADMIN_LASTNAME}" \
+      --email "${ADMIN_EMAIL}" \
+      --password "${ADMIN_PASSWORD}"
 fi

Review Comment:
   **Suggestion:** With `/bin/sh`, the pipeline status is the status of `grep`, 
not `superset fab list-users`. A database or CLI failure from `list-users` is 
therefore treated as a non-match and the script proceeds to `create-admin`, 
while the original error is suppressed by `2>/dev/null`. This can produce a 
misleading creation error or create an account despite an unavailable metadata 
database. Preserve and fail on the `list-users` error before attempting 
creation. [possible bug]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Database/listing failures are misreported during initialization.
   - ❌ Init Jobs may attempt account creation after failed discovery.
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3ab3569629664ac3b4b291601c748cf0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=3ab3569629664ac3b4b291601c748cf0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   
   *(Use Cmd/Ctrl + Click for best experience)*
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** helm/superset/templates/_helpers.tpl
   **Line:** 719:728
   **Comment:**
        *Possible Bug: With `/bin/sh`, the pipeline status is the status of 
`grep`, not `superset fab list-users`. A database or CLI failure from 
`list-users` is therefore treated as a non-match and the script proceeds to 
`create-admin`, while the original error is suppressed by `2>/dev/null`. This 
can produce a misleading creation error or create an account despite an 
unavailable metadata database. Preserve and fail on the `list-users` error 
before attempting creation.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve 
this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask 
user if the user wants to fix the rest of the comments as well. if said yes, 
then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42753&comment_hash=7d6fe642e990b5aeb071a06cb3abd791c6b03d522ca9b8d13c26477ee27ca0d9&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42753&comment_hash=7d6fe642e990b5aeb071a06cb3abd791c6b03d522ca9b8d13c26477ee27ca0d9&reaction=dislike'>👎</a>



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to