Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v16] Move Server Scripts to the file system #26385

Open
rmehta opened this issue May 9, 2024 · 1 comment
Open

[v16] Move Server Scripts to the file system #26385

rmehta opened this issue May 9, 2024 · 1 comment

Comments

@rmehta
Copy link
Member

rmehta commented May 9, 2024

Why

No matter how much we secure, server scripts will be vulnerable to edits since they directly live in the DB. Server Scripts are absolutely necessary, so we can't kill them either. Maybe we need to move them to a different place which is outside Frappe API. Also RestrictedPython is lame

How

  1. Move server scripts to the filesystem (maybe a /scripts) folder in the /sites
  2. Maybe the DocType remains as-is, only the code sits outside the DB and can't be edited from the desk interface. (We can show a read-only version in the DocType)
  3. Editing can only happen if you have SSH / SFTP access on that folder - can also be a nice feature in Press to edit it.
  4. Include scripts in backups

More

Should also apply to reports, print templates, email templates

cc: @sagarvora @ankush @akhilnarang

@rmehta rmehta changed the title Move Server Scripts to the file system [v16] Move Server Scripts to the file system May 9, 2024
@ankush
Copy link
Member

ankush commented May 9, 2024

Slightly modified proposal:

  • Leave old doctypes as is, introduce new doctype Extension Scripts or just Extensions (naming can be dealt with later)
  • Script types based on folder structure
- /scripts
    - /doctype_events
        - Sales Order.py (all events moved inside single file like controllers)
    - /api
        - path.py
    - /cron
        - crontab (cron -> file.py) 
        - script.py

Execution:

  • Use exec directly instead of safe_exec i.e. allow everything.
  • Not allowed in shared environment (same as current server script v15)
  • Provide a flag for using old syntax OR write a code-rewriter for old script to migrate to normal python syntax instead of safe_exec ones (quite hard IMO)
  • Once this is ready and we are confident about it, migrate server script contents to file and disable old doctype.

Pros:

  • better security model
  • version controllable
  • full access to vanilla python, libraries, imports

Cons:

  • Without a UI editor this can be quite a pain for users to edit using SSH. This can be solved by FC. For Frappe docker we can provide VS Code container with some control over who can access it. ( @revant 👀 )
  • Allowing full python also means allowing monkey patching using this, which eventually bigbraindevs will do. 🌚
  • breaking & migration woes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants