fix: self-heal Flash backend service-account role grants on migrate#54
Merged
Merged
Conversation
The Flash Admin + Accounts Manager role *assignments* on the backend service-account User are not versioned (unlike the DocPerm role *definitions*, which ship in doctype JSON). They silently dropped once this week, which 403'd cashout and the account-upgrade flow until re-granted by hand. Add ensure_service_account_roles() to after_migrate so the assignment re-asserts on every `bench migrate`: - Hardcoded 2-email candidate list (flash_sa@ prod / flash-service-account@ test); whichever exists on the site gets the roles, the other is absent. A `flash_service_account` site_config key overrides without a code change. - Idempotent: only calls add_roles() when a role is actually missing, so a converged site does no needless User.save() per migrate. - Deliberately does NOT auto-grant based on "any user with an api_key" — live data shows non-service users hold api_keys too, so that heuristic would be a privilege-escalation vector. Acceptance-tested on TEST: dropped Flash Admin from the service account, ran the function, confirmed it restored the role and was a no-op on re-run. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
The Flash backend service account needs two ERPNext roles to serve cashout and the account-upgrade flow:
Role definitions ship in doctype JSON and are versioned/safe. Role assignments on the User record are not versioned — and this week the
Flash Adminassignment silently dropped, 403'ing cashout and the upgrade flow until an operator re-granted it by hand. Nothing in the app re-asserted it.after_migrate()already calledensure_roles(), but that only creates theFlash AdminRole record — it never assigns it to the service account. That was the gap.Fix
Add
ensure_service_account_roles()toafter_migrate, so the assignment re-asserts on everybench migrate(i.e. every deploy):flash_sa@getflash.io(prod) andflash-service-account@getflash.io(test). Whichever User exists on the site gets the roles; the other is simply absent. Aflash_service_accountkey in site_config overrides the list without a code change.add_roles()for a role that's actually missing, so a converged site does no needlessUser.save()per migrate.Placed in
after_migrate(runs every migrate, correct for a self-healing invariant) rather than a Frappe patch (runs once via patch_log — wrong for self-heal).Testing
Acceptance-tested on TEST ERPNext:
Accounts Manager, Flash AdminAccounts Managerensure_service_account_roles()Accounts Manager, Flash Admin✓Accounts Manager, Flash Admin✓ (no write)python3 -m py_compileclean; no mixed tab/space indentation.Deploy
Ships on the next ERP image tag →
values.base.yamlbump →make erp. The self-heal takes effect during that deploy'sbench migrate.🤖 Generated with Claude Code