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>
[](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)
[](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>
[](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)
[](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>
[](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)
[](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]