REQUEST now supports POST

[johncant]

The experimental REQUEST function now supports any HTTP method.

This makes sense if

• Your formula is idempotent, taking actions that make external state consistent with the data in your document
• You have an endpoint with GET behaviour, but it actually requires a POST/PUT method

[fflorent]

If the backend handles the POST requests, wouldn't a malicious user be able to execute commands to other services with the IP of the server? (making a security breach)

[paulfitz]

Looks about right, some comments, thanks @johncant.

My main concern is whether this could come as a surprise to other Grist self-hosters. GRIST_EXPERIMENTAL_PLUGINS would need a stronger warning now I think, since turning it on will result in a quite exploitable system (especially if GRIST_HTTPS_PROXY is not set to at least protect internal endpoints). Perhaps a new flag would be the way to go, for example a separate GRIST_ENABLE_REQUEST_FUNCTION? I'd be ok with surprising some users with the method stopping working until they find and enable this flag - the existing "experimental" flag suggests flux is possible.

[johncant]

If the backend handles the POST requests, wouldn't a malicious user be able to execute commands to other services with the IP of the server? (making a security breach)

Yes, although it depends on who can access your Grist and what internal endpoints are accessible from the Grist backend. Also true for GET requests, but this PR widens the potential for damage. I'd agree that GRIST_ENABLE_REQUEST_FUNCTION flag, disabled by default, and with some documentation, would solve this problem.

@paulfitz @alexmojaki I could implement when I have time, also address your comments.

[fflorent]

I didn't know the function were disabled by default. That solves my main concern :).

Perhaps a new flag would be the way to go, for example a separate GRIST_ENABLE_REQUEST_FUNCTION?

I wonder if there wouldn't be a way to name this function to be future-proof, in case other possibly-harmful functions would be introduce (unless you say it's much probably the only function of that sort). In that case, I would suggest to introduce a variable whose value would not be a boolean but a list of function names (GRIST_..._FUNCTIONS="request").

Though I am not sure about how to name them (GRIST_EXTRA_FUNCTIONS_USE_WITH_CAUTION="request"? GRIST_REMOTE_EXECUTION_FUNCTIONS_USE_WITH_CAUTION="request"?).

[johncant]

It makes no difference to me either way - will go with GRIST_ENABLE_REQUEST_FUNCTION for now and change depending on outcome of any discussion :)

[paulfitz]

At Grist Labs we are not planning on introducing several functions like this. REQUEST is a stalled project, something that got started but foundered on some complications - security questions, and concern that naive users could end up doing a LOT of requests. Glad to see it getting use.

[johncant]

@paulfitz this makes sense - I can't imagine a way of making REQUEST work in an environment where the users aren't trusted and could be naive and expect speed, that doesn't involve a lot of complexity. It's a killer feature though for integrating Grist with things, so thanks a lot!

This PR is now ready to be re-reviewed.

I've manually tested that GRIST_ENABLE_REQUEST_FUNCTION=1 is needed to enable REQUEST, also some simple examples of GET and POST with actual Grist.

[alexmojaki]

It's a killer feature though for integrating Grist with things, so thanks a lot!

REQUEST is my baby, so thanks for saying this, it warms my heart 😊

[johncant]

Ok - this PR is now ready for re-re-review.

[alexmojaki]

LGTM, thanks! @paulfitz are you satisfied?

[paulfitz]

LGTM, thanks! @paulfitz are you satisfied?

Yup, thank you both!