feat(database-credentials): add password update functionality for Mar…#4144
Conversation
…iaDB, MongoDB, MySQL, Postgres, and Redis - Introduced a new `UpdateDatabasePassword` component to facilitate password updates for database credentials. - Implemented password change mutations in the respective API routers for MariaDB, MongoDB, MySQL, Postgres, and Redis. - Enhanced user experience by providing success notifications upon successful password updates. - Updated UI components to include the new password update functionality, ensuring consistency across different database types.
dosubot
bot
added
the
size:L
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| echo "No running container found for ${appName}" >&2 | ||
| exit 1 | ||
| fi | ||
| docker exec "$CONTAINER_ID" redis-cli -a '${databasePassword}' CONFIG SET requirepass '${password}' |
There was a problem hiding this comment.
databasePassword and password
Both the existing password (used for -a '${databasePassword}' authentication) and the new password ('${password}') are interpolated without escaping. A password containing a single quote will break out of the single-quoted shell context. Additionally, using -a to pass the password on the command line will cause Redis to print a deprecation warning; REDISCLI_AUTH env var or --pass via stdin is preferred, but more importantly the values must not be shell-interpolated raw.
| const command = ` | ||
| CONTAINER_ID=$(${containerCmd}) | ||
| if [ -z "$CONTAINER_ID" ]; then | ||
| echo "No running container found for ${appName}" >&2 | ||
| exit 1 | ||
| fi | ||
| docker exec "$CONTAINER_ID" psql -U ${databaseUser} -c "ALTER USER \\"${databaseUser}\\" WITH PASSWORD '${password}';" | ||
| `; |
There was a problem hiding this comment.
password interpolation
The password value (and databaseUser) are interpolated directly into a shell string using single-quote delimiters. A password that contains a single-quote character will prematurely terminate the shell string, allowing an attacker to inject and execute arbitrary shell commands on the host running the container.
The same issue exists in the equivalent changePassword mutations in mariadb.ts (~line 400), mongo.ts (~line 419), mysql.ts (~line 421), and redis.ts (~line 406) — all five routers are affected.
The recommended remediation is to avoid interpolating secrets into shell strings at all. One approach is to write the sensitive value to a file or pipe it inside the container via docker exec -i, so it is never part of the evaluated shell string. At a minimum, single-quote characters in any interpolated value must be escaped before embedding in a single-quoted shell argument.
| const authPassword = | ||
| type === "root" ? databaseRootPassword : databaseRootPassword; |
There was a problem hiding this comment.
Both arms of the ternary expression evaluate to databaseRootPassword, so authPassword is always the root password regardless of type. Using root credentials to authenticate is actually correct (root is required to run ALTER USER for any MySQL account), but the ternary is misleading dead code — databasePassword is never even destructured from my.
Simplify to a direct assignment to make intent explicit. The variable authPassword should just be set to databaseRootPassword unconditionally.
| const updatePasswordSchema = z.object({ | ||
| password: z.string().min(1, "Password is required"), | ||
| }); |
There was a problem hiding this comment.
The form only collects the new password once. A typo in the new password will silently lock the database with an incorrect credential. Adding a "Confirm password" field and validating that both values match (e.g. with a Zod refine check) would prevent this class of user error.
…date failures - Improved error messages when updating the database password to provide clearer guidance based on the error type. - Added specific feedback for cases where the database container is not running, prompting users to start the service before attempting to change the password.
…atabase routers - Added confirmation password field and validation to the `UpdateDatabasePassword` component. - Refactored password update logic in MariaDB, MongoDB, MySQL, Postgres, and Redis routers to utilize database transactions for improved reliability. - Ensured consistent handling of password updates across all database types, enhancing user experience and security.
dosubot
bot
added
size:XL
…e routers - Updated password validation in MariaDB, MongoDB, MySQL, Postgres, and Redis routers to enforce a regex pattern that restricts invalid characters. - Introduced a consistent error message for invalid passwords to improve user guidance and ensure database compatibility. - Refactored password validation logic in the schema files to utilize shared constants for regex and messages, promoting code reuse and maintainability.
Siumauricio
deleted the
4041-add-ability-to-change-services-password-like-mysql-and-whatnot
branch
1 participant
…iaDB, MongoDB, MySQL, Postgres, and Redis
UpdateDatabasePasswordcomponent to facilitate password updates for database credentials.What is this PR about?
Please describe in a short paragraph what this PR is about.
Checklist
Before submitting this PR, please make sure that:
canarybranch.Issues related (if applicable)
closes #4041 #933
Screenshots (if applicable)
Greptile Summary
This PR adds a live password-update feature for all five database types (MariaDB, MongoDB, MySQL, Postgres, Redis). A new shared
UpdateDatabasePassworddialog component collects the new password, calls a per-databasechangePasswordtRPC mutation, and invalidates the query cache on success. Each mutation shells out to the running container viadocker execto change the password at the DB level, then persists the new value in Dokploy's own store.The UI integration and the overall flow are well-structured, but the server-side implementations have critical security and correctness issues that need to be resolved before merging:
passworddirectly into single-quoted shell strings without any escaping. A password containing a shell metacharacter would break out of the quoted context, potentially executing arbitrary commands on the host. This affectspostgres.ts,mysql.ts,mariadb.ts,mongo.ts, andredis.ts.mysql.ts(P1) —authPasswordis computed with a ternary where both branches evaluate todatabaseRootPassword, making the conditional pointless and the code misleading.UpdateDatabasePassworddialog accepts only a single password input. A typo will silently lock the database. Adding a confirmation field with a Zodrefinecheck would prevent accidental lockouts.Confidence Score: 1/5
Not safe to merge — the shell injection vulnerability allows an attacker who can set a database password to execute arbitrary commands on the host.
All five new server-side mutations contain a shell injection vulnerability that lets a crafted password break out of the single-quoted shell string and run arbitrary commands on the host. This is a critical security issue. There is also a dead-code ternary bug in mysql.ts and a minor UX gap in the shared component. These issues must be resolved before the PR is merged.
All five router files (postgres.ts, mysql.ts, mariadb.ts, mongo.ts, redis.ts) need the shell injection fixed. mysql.ts also needs the dead-code ternary removed.
Reviews (1): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile