Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughServer mode now uses administrator-configured password commands by name. The change adds command-name storage, startup conversion of legacy values, shared-server inheritance and overrides, and mode-specific API, browser, import, and export handling. Commands run without a shell in a reduced environment; Desktop mode retains free-text command handling. ChangesNamed Server Password Execution
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant ServerManager
participant build_passexec
participant resolve_server_passexec
participant ServerPasswordExec
participant subprocess
ServerManager->>build_passexec: Build password executor
build_passexec->>resolve_server_passexec: Resolve the user's command
resolve_server_passexec-->>build_passexec: Return command name and connection details
build_passexec->>ServerPasswordExec: Create executor with configured argv
ServerPasswordExec->>subprocess: Run argv without a shell
Merge Risk: ⚪ Minimal · up to Server-mode password commands now come from an administrator-defined allowlist and run without a shell, in a reduced environment. No concrete merge-blocking risk was identified in the supplied context. Administrators must move any existing commands into the new setting before the first start after upgrading. Pre-merge checks |
|
|
Note Automated review by an AI agent (Claude Fable 5.1), not by @dpage. Dave asked a Claude Code agent with a clean context to review this PR; the findings below are the agent's own and have not been edited by Dave. This does what it says: in server mode the free-text command path is gone from every entry point I could find (create, update, import, startup conversion and the Security points I checked and am satisfied with
Non-blocking
Note on verification: the branch is 25 commits behind |
…e owner's on shared servers.
…ument the allowlist.
…s from the command line.
…osing the command's arguments.
… an administrator passthrough list.
…ad of an inherited one.
…er users' control.
…s not block other users.
…mmands are configured.
…ssword exec upgrade.
…ion warnings by row.
…T in the reduced environment test on Windows.
…uns, since patching SERVER_MODE on a desktop app breaks its login checks.
…EC_COMMANDS is configured, and document that it must be set before the first start after upgrading.
687a743 to
c30cfd8
Compare
|
Note Changes made by an AI agent (Claude Fable 5.1), not by @dpage. Dave asked a Claude Code agent to fold in the non-blocking points from the automated review above. Point 1: Rebased onto current |
…rator allowlist. (#10506) In server mode, users choose a password exec command by name from SERVER_PASSEXEC_COMMANDS rather than entering a command line. Existing commands are converted on upgrade once the allowlist is configured; if it is empty the conversion is postponed and a warning is logged.
|
Merged to master as c41b606. |
In server mode,
PasswordExecCommandwas free text run through a shell, so anyone able to edit a server could run arbitrary commands as the pgAdmin service account. This replaces it with a list of named commands that the administrator defines inconfig.py, which users can only pick from. Desktop mode is unchanged. Addresses #10249, and replaces #10328.What changes in server mode
SERVER_PASSEXEC_COMMANDSmaps names to argv lists. Commands run withshell=False, and the connection details reach them through thePGADMIN_PASSEXEC_HOST,_PORT,_USERNAME,_DATABASE,_PGADMIN_USERand_AUTH_SOURCEenvironment variables rather than placeholder substitution.PATH,HOME, locale and similar, plus anything named in the newSERVER_PASSEXEC_ENV_PASSTHROUGH), so other users'PGPASSWORDor container setup passwords never reach them.passexec_cmdis never run in server mode.ENABLE_SERVER_PASS_EXEC_CMDis deprecated and logs a warning.servers.jsonimport and export carry the command by name.Upgrade impact: administrators who used
ENABLE_SERVER_PASS_EXEC_CMDmust list their commands inSERVER_PASSEXEC_COMMANDS, and commands relying on%HOSTNAME%-style placeholders or on environment variables outside the base list need adjusting. The upgrade notes inserver_dialog.rstcover this.The server dialog changed, so
run-feature-tests-pgis the check to watch.Summary by CodeRabbit