Repository navigation
Conversation
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the previously reported alias bypass is fixed and covered by regression tests. Reviews (4): Last reviewed commit: "fix(cli): Guard command aliases, not jus..." | Re-trigger Greptile |
On Frappe Cloud, Press owns the Bench and Site records and rebuilds the container from an image on every deploy, so commands run by hand inside the container silently drift the two apart. Prompt before those commands when the bench's common_site_config carries the frappe_cloud marker. The check runs before frappe passthrough commands are execv'd, since the process is replaced there, and skips when stdin isn't a TTY so the agent's docker exec calls are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
regdocs
force-pushed
the
feat/frappe-cloud-prod-guard
branch
from
August 6, 2026 11:01
b439af6 to
4d6d75a
Compare
MultiCommandGroup files one command object under several names, so `bench get`, `bench remove` and `bench rm` reach get-app and remove-app without matching the guard table. Those are exactly the app changes the table exists to catch, and they were passing through unwarned. Resolve aliases by identity against the registered commands rather than listing them, so an alias added later is covered without a table edit. Point at the dashboard docs while we're here: each entry is now a (reason, docs_url) pair -- get_guard_reason becomes get_guard -- and the warning prints the page that documents doing the same thing from the Frappe Cloud dashboard. `setup` has no dashboard equivalent, so its link is None and the line is omitted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
regdocs
force-pushed
the
feat/frappe-cloud-prod-guard
branch
from
August 11, 2026 07:04
86b5690 to
fb33a7c
Compare
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Problem
On Frappe Cloud, the bench inside the container is a replica, not the source of truth. Press owns the Bench and Site records and rebuilds the container from an image on every deploy. Commands run by hand over SSH drift the two apart, and the drift is silent:
bench --site x install-app erpnext— the dashboard still shows the old app list, and the app is gone after the next deploybench --site x drop-site— Press keeps showing the site as active, and keeps billing itbench get-app/bench update— discarded on the next deploy, and the recorded app hashes no longer matchbench set-config/bench config— overwritten on the next config pushEach of these turns into a support ticket that starts with "the dashboard is wrong".
What this does
Prompts before those commands and explains which record goes out of sync:
Declining exits non-zero. Accepting runs the command — the guard is advisory, not a lock.
When it fires
All of these must hold, so it's inert everywhere except an interactive shell on a managed bench:
GUARDED_COMMANDS, or is an alias of onebackup,console,mariadb,build,doctor,clear-cacheetc. are untouchedfrappe_cloudincommon_site_config(orFRAPPE_CLOUDenv)sys.stdin.isatty()docker execwith no-t, so automation passes through untouchedBENCH_SKIP_PROD_GUARDunset--help/-hin argvbench update --helpshouldn't prompt for a command it never runsWhere the hook lives
In
cli(), after the--help/no-args early return and before both dispatch paths. This placement is load-bearing: commands not registered with bench (everything--site-scoped) are handed to frappe viaos.execv, which replaces the process. Anything downstream of that can't prompt.Guarded commands
Site state (tracked on the Site record):
new-site,drop-site,install-app,uninstall-app,set-config,remove-config,restore,reinstall,migrate,rename,set-admin-passwordBench state (tracked on the Bench record / Release Group):
get-app,new-app,remove-app,update,switch-to-branch,switch-to-develop,retry-upgrade,pip,setup,config,start,restart,dropAliases are resolved from the command registry, not listed in the table:
MultiCommandGroupfiles one command object under several names, sobench get(=get-app) andbench remove/bench rm(=remove-app) are covered, and an alias added to a guarded command later needs no table edit. A test walks the registry and fails if any alias of a guarded command is unguarded.Notes
No
--yesflag. frappe'sreinstallalready defines its own--yes, and unregistered commands areexecv'd to frappe verbatim — a bench-level flag would either collide or have to be stripped from argv, changing whatbench --site x reinstall --yesmeans.BENCH_SKIP_PROD_GUARD=1is the bypass instead, and the abort message names it.No agent changes needed. Verified that
agent/bench.pybuildsdocker execwithout-t, so agent-issued commands are non-interactive and skip the prompt.Testing
15 unit tests in
bench/tests/test_prod_guard.pycover detection, the command table, and every skip path.Manually against a real bench:
FRAPPE_CLOUD=1+install-app, declined → exit 1, never reached frappeinstall-app --help→ help printed, no promptCompanion PR
Requires frappe/press#7130 to write the marker. This PR is a no-op without it.
🤖 Generated with Claude Code