[Feature]: Make RESTRICTED_DOCTYPES configurable, least-privilege MCP users cannot read Server Script and other admin DocTypes at all

[Carlo-SR]

Description

RESTRICTED_DOCTYPES in frappe_assistant_core/core/security_config.py is a hardcoded blocklist that cannot be configured, and only System Manager bypasses it. For anyone running a least-privilege integration user, this makes DocType access all-or-nothing: either the MCP user gets System Manager (full read, write, delete, plus run_database_query), or a large set of DocTypes is invisible to it no matter what the Frappe permission model says.

Please make this list configurable, or better, let Frappe's own DocType permissions decide read access and keep the hard block only for the write side of code-execution DocTypes.

What the code does today

1. get_user_primary_role() only recognizes three roles: System Manager, Assistant Admin, Assistant User. Every other user resolves to the string "Default", so custom roles are invisible to this layer.
2. is_doctype_accessible() returns True unconditionally for System Manager. For everything else it does:

   role_to_check = user_role if user_role in RESTRICTED_DOCTYPES else "Assistant User"

   RESTRICTED_DOCTYPES has exactly one key, "Assistant User", so both "Default" and "Assistant Admin" fall back to the Assistant User blocklist. Promoting the user to Assistant Admin changes nothing.
3. validate_document_access() runs this check before any frappe.has_permission() call, so granting a role read permission on the DocType has no effect at all.

The blocklist covers, among others: Server Script, Client Script, Custom Script, DocType, DocField, Custom Field, Property Setter, Customize Form, Custom DocPerm, Role, User Permission, Workflow, plus all log DocTypes.

Verified on FAC 2.3.1 (Frappe 15.91.3, ERPNext 15.92.5) and re-checked against main at 7ddc433 (2026-08-20), where the logic is unchanged and still wired into get_document, list_documents, create_document, update_document, submit_document and chatgpt_fetch. Unlike ROLE_TOOL_ACCESS / BASIC_CORE_TOOLS, which you described as legacy in #216, this code path is live.

Concrete symptom with a low-privilege user:

list_documents(doctype="Server Script")   -> "Access to Server Script is restricted for your role"
fetch(id="Server Script/<name>")          -> "Permission denied: Access to Server Script is restricted for your role"
run_database_query("SELECT ... FROM `tabServer Script`")
                                          -> "Insufficient permissions. System Manager role required for query execution."

Use Case

We run FAC against a production ERPNext site and deliberately never connect it as a privileged user. Our setup is one dedicated integration user per environment with our own roles (MCP Read Only, MCP Write new Docs only, and so on), granting read on a wide set of DocTypes and write on very little. That is the whole point of the pattern: the assistant should be able to see a lot and change almost nothing.

The automations on that site are Server Scripts, Client Scripts, Custom Fields and Property Setters. Reviewing them, documenting them, checking which script touches a field, or diffing a production script against the test site is exactly the kind of read-only work we want the assistant to do. Today that is impossible without handing the same user System Manager, which also hands it delete rights and arbitrary SQL. So the safe option is to give up the feature, and the useful option is to give up the safety model.

To be explicit about what we are asking for: read access driven by our roles. We are not asking to write Server Scripts over MCP.

Proposed Solution

Options in order of preference:

1. Let Frappe permissions decide read access. Drop the read-side blocklist and rely on frappe.has_permission(doctype, "read"), which is already called further down in validate_document_access(). If a site does not want its MCP user reading Server Script, it simply does not grant read on Server Script. This is the same reasoning you used in Feature: configurable role/user access for report tools + create read-only reports without System/Report/Script Manager #216 for the Report DocType: FAC should not second-guess the permission model it already defers to.
2. Make the list configurable, in the same spirit as FAC Tool Configuration. For example a child table on Assistant Core Settings, or a FAC DocType Access DocType, with a mode per role: Use Frappe Permissions / Use Built-in Blocklist / Custom List, defaulting to the current behaviour so nothing changes for existing installs.
3. Minimal version if the above is too much, one settings field Additional Allowed DocTypes that subtracts entries from RESTRICTED_DOCTYPES at runtime, so an admin can opt in per DocType without patching the app.

Two details worth keeping in any variant:

• Split read from write. Hard-blocking create / update / delete on code-execution DocTypes (Server Script, Client Script, Property Setter) is defensible and we would not object. Blocking read is what hurts. The current single list does not make that distinction.
• Make get_user_primary_role() aware of custom roles, or stop routing this decision through a single "primary role" string. Collapsing every non-assistant role to "Default" means site-specific roles can never express intent here.

Alternatives Considered

• Grant System Manager to the MCP user. Works, defeats the purpose. It also unlocks run_database_query and every write tool on a production site.
• Add Assistant Admin. Does not help, see point 2 above: it falls back to the same Assistant User blocklist.
• Grant read DocPerms on Server Script. No effect, the blocklist is evaluated before the permission check.
• Patch security_config.py on the site. Not viable for us, the instance is hosted and the change would be lost on the next app update.
• Copy the script bodies out of the browser by hand. What we do now. It scales badly and the copy goes stale immediately.

Side observation: the blocklist is not a consistent boundary

While tracing this, the enforcement looked uneven, which may be worth a separate look:

• validate_document_access() is only called by get_document, list_documents, create_document, update_document, submit_document and chatgpt_fetch.
• delete_document.py never calls it. It relies purely on frappe.has_permission(doctype, "delete"), which is the model we are asking for on the read side.
• search_doctype, search_documents, search_link, get_doctype_info and the report tools do not consult the list either.

Empirically, on our production site with the same low-privilege user that is refused by list_documents, search_doctype on Server Script happily returns document names, api_method, cron_format and our custom fields. Only the script field stays out of reach. So the blocklist does not really contain anything, it mainly blocks the well-behaved read path while leaving the search path open. That is another argument for letting the permission model handle it in one place.

[buildswithpaul]

Thanks — this is an exceptionally well-traced issue, and every claim in it checks out. Fixed in #250, going to develop.

I went with your option 1 plus the read/write split you suggested, and no new config surface. Here's the reasoning, because one thing I found while verifying makes the decision much easier than it looks.

The blocklist was already a no-op by default

I pulled the stock DocPerms out of Frappe v15 for the DocTypes on the list — DocType, Server Script, Client Script, Custom Field, Property Setter, Role, User Permission, Custom DocPerm, DocShare, Error Log, Activity Log, Access Log, View Log, System Settings, Print Settings, Email Queue, Workflow, Integration Request, Data Import.

Every single one grants read to System Manager / Script Manager / Administrator only.

So on a stock install the read blocklist never changed an outcome — the user was refused one line later by frappe.has_permission anyway. It only ever fired where a site admin had deliberately granted read via a Custom DocPerm. That is, it was dead weight in the default case and an override of explicit intent in yours. Which is the value proposition exactly backwards, and it settles the "but is dropping it safe" question: for everyone except sites doing what you're doing, this changes nothing.

Your side observation was right too, and it's the other half of the argument: delete_document deletes what get_document refuses to display, and the search path returns the same rows under frappe.has_permission alone. It wasn't containing anything.

What changed

RESTRICTED_DOCTYPES is replaced by WRITE_PROTECTED_DOCTYPES — 21 DocTypes that carry executable code or define the schema and permission model. create/write/delete/submit/cancel/amend on those stay blocked regardless of DocPerms. Everything non-mutating defers entirely to frappe.has_permission().

• Your case works. A role with a read-only Custom DocPerm on Server Script gets get_document, list_documents and fetch, script body included. I built your exact setup — custom role, integration user, read-only DocPerm — and verified it against both revisions.
• Writes stay blocked, as you said you'd expect. The error is now specific: "Write access to Server Script is blocked over MCP because it defines executable code, schema or permissions".
• The role collapse is gone. The list is no longer keyed by role at all, so the Assistant Admin → Assistant User fallback you found can't recur. I didn't change get_user_primary_role() itself — with reads now permission-driven there's nothing left for a "primary role" string to decide on this path, and it still drives field masking where changing it would have side effects. If the "Default" collapse bites you somewhere else, open a separate issue and I'll look at it properly.
• delete_document now goes through the same guard, so the boundary is consistent across all seven document tools.
• System Manager keeps its bypass, so installs that legitimately write Custom Fields over MCP aren't broken by this.
• RESTRICTED_DOCTYPES survives as a deprecated alias in case anything external imports it.

No new settings field. Read access is now driven by your roles, which is what you asked for, and I'd rather not ship a config knob whose only remaining purpose would be to weaken the write block.

One caveat worth knowing about

ADMIN_ONLY_FIELDS has wholesale "*" hides for System Settings, Print Settings, LDAP Settings and friends, but filter_sensitive_fields() only applies them when the role resolves to exactly "Assistant User" — so a custom role like your MCP Read Only (which resolves to "Default") gets less field masking than an Assistant User would. This change makes that asymmetry reachable on DocTypes it previously couldn't reach.

The genuinely secret fields there — encryption_key, backup paths — are masked by SENSITIVE_FIELDS, which applies to every non-System-Manager role, so nothing sensitive leaks. But if you grant your roles read on those settings DocTypes, be aware you'll see more of them than the naming suggests. I left it alone deliberately: widening the "*" rule would re-introduce the same deny-an-explicit-admin-grant pattern this change removes, so it deserves its own decision rather than a drive-by.

On #216

Fair callout on the distinction. ROLE_TOOL_ACCESS/BASIC_CORE_TOOLS really are dead — I confirmed check_tool_access and get_allowed_tools have zero callers — but you were right that this path was live and I shouldn't have left the two conflated. is_doctype_accessible is now documented as what it actually is: a write guard that can only ever deny what Frappe would allow, never grant.

Full suite is green (352 tests) with 10 new ones covering the read pass-through and the write block. Happy to have you try the branch against your test site before it merges — improvement/249-doctype-write-protection.