Clnrest: dynamic paths

[daywalker90]

based on #7507, #7508 and #7509

Adds the capability for clnrest to handle dynamic paths registered via a plugin's manifest. Supports the http methods GET and POST and arbitrary content-type's for responses. The most specific path is chosen if there are multiple matches.

lightningd is responsible for making sure no ambigous paths like /path/to/<me> and /path/to/<you> are registered.

[ShahanaFarooqui]

@daywalker90 Thank you for the PR! I’m moving it to the next milestone since it enhances our current clnrest plugin. For this release, our priority is to smoothly replace the Python plugin with the Rust version. Once that’s done, we can introduce this and other enhancements in future releases. Apologies for the delay!

[rustyrussell]

@daywalker90 Can you please rebase so we can finally merge this, if you're still happy?

[daywalker90]

Ok so i rebased this but it's not ready to merge. niftynei's feedback regarding versioning is not yet addressed, i remember vividly that there was something wrong with the rust code aswell but can't remember and i don't even know what gudnuf didn't finish in #7507.

So if someone could review the C + pyln-client code and add this part from the feedback:

add an optional "version" field which defaults to the current CLN rest one. it can be set to 'null' which removes it entirely.

that would help alot.

Remember i was contacted by gudnuf with this, i just wanted to do the rust side of clnrest, not design the new manifest. I don't know what people need...

[ShahanaFarooqui]

So if someone could review the C + pyln-client code and add this part from the feedback:

Assigned it to @rustyrussell as he agreed to review (and update if needed) the C and pyln-client code.

[rustyrussell]

Re v1/ path:

1. We can simply stick with v1 today. Handling other urls is tomorrow's problem
2. Make sure the custom handler gets the whole path, including /v1/. That way they don't have to change anything if we start supporting arbitrary prefixes in future.

But more importantly, I think you don't need to put the clnrest path in the manifest; that's a bit weird. Just implement a "clnrest-register-path" call directly in clnrest! Points:

1. You can certainly make the pyln wrapper do this call automatigically during init as a courtesy.
2. Make sure the path starts with /v1/ (and fail it for now if it doesn't)
3. Make sure clnrest can handle calls to clnrest-register-path before init (save them up until after init), because this could actually happen.
4. Not sure if python plugins should fail gracefully if the call fails. That way they automatically disable themselves if clnrest is disabled for some reason?

[daywalker90]

3. Make sure clnrest can handle calls to clnrest-register-path before init (save them up until after init), because this could actually happen.

I don't know how to do that, pretty sure this requires some changes to cln-plugin aswell?

If i could do that the "clnrest-register-path" call could take in everything and we could drop the whole manifest approach. But that's why we chose that approach because nobody thought we could catch all pre-init calls.

[rustyrussell]

3. Make sure clnrest can handle calls to clnrest-register-path before init (save them up until after init), because this could actually happen.

I don't know how to do that, pretty sure this requires some changes to cln-plugin aswell?

Actually, here's how it works at startup:

1. We call getmanifest on everyone, wait until they all respond.
2. We call init on everyone at once.

This means you will never get called before init if you're not dynamically loaded (the call will get queued behind init), BUT you can get called before init returns (if you're doing async stuff, for example). (Note: there's an important exception for db hooks, which are called from lightningd itself, but they're magic and special).

If i could do that the "clnrest-register-path" call could take in everything and we could drop the whole manifest approach. But that's why we chose that approach because nobody thought we could catch all pre-init calls.

Yeah, understood. But I think it mainly "just works".

[madelinevibes]

I would love to see this merged for the cln release early H1 2026....

[daywalker90]

Ok i took in all the feedback and noticed: hey! i can do that.

• Everything back to 0: no more manifest stuff
• new "clnrest-register-path" rpc method for clnrest

The new command takes a path, rpc_method and optionally a rune argument, where rune is just an object of the checkrune arguments nodeid, method and params and is passed to checkrune. No rune argument -> always authorized! Conflicting path's are not allowed. I could change this, but then they could override each others paths. Note that any rpc caller can override CLN's paths by setting path's like this: v1/pay since they are "unregistered"! The version part is now also passed to the handler so people can freely choose what, if any, version should be part of their path. It is easier and more precise for rune to be a dictionary, since we can get key:value pairs from both a capture path like /mint/{user} aswell as the request body and if it's an array noone knows (right?) what should come first, hence the msggen commit for a string_map.

I dropped both content_type aswell as (http) method since i didn't see a good use for them. I could check if the current request is using the specified content_type and method but after that they get treated the exact same. clnrest supports json, yaml, xml and url-form encoded content_type's and you can freely pick one of them. If you can think of a good reason as to what i should do with them, please let me know.

I had to rearrange some routing logic to make it all work and after finally understanding that axum layers are like onions where the last layer gets applied first. That means unauthorized socket.io connections get rejected a little earlier during the connect.

PR also includes a little refactoring of structs to a new file and some minor drive-by fixes of unnecessary clones and deprecated dependencies.

Let me know what you think!

[daywalker90]

Oh and about this:

I don't know how to do that, pretty sure this requires some changes to cln-plugin aswell?

If i could do that the "clnrest-register-path" call could take in everything and we could drop the whole manifest approach. But that's why we chose that approach because nobody thought we could catch all pre-init calls.

turns out

it mainly "just works"

[daywalker90]

I've changed the behaviour of not setting the rune parameter as per the discussion in the CLN meeting.

If you call clnrest-register-path without rune it now defaults to using the matched rpc method aswell as the rpc params, matching the behaviour of calling normal CLN methods more closely.

It was also discussed to not add any methods to delete routes in this PR and saving routes is also out of scope for this PR.

The question remains if the clnrest plugin should stay dynamic?

[daywalker90]

Thank you for your feedback, Shahana!

Dynamic plugin loading makes REST functionality optional, ensuring zero resource consumption for users who don’t require REST capabilities. For now, plugins that require full readiness before handling messages can work around this by awaiting all asynchronous initialization before returning from init. We can revisit this approach if a compelling use case arises in the future.

Right, i was just thinking about other plugins. They need to deal with a suddenly appearing/disappearing clnrest if it stays "dynamic" and we don't remember their dynamic routes.

Documentation location

You are talking about the clnrest-register-path.json file, correct? I don't mind either way, if it's not as a json schema users of cln-rpc won't get a typed call. No that big deal of a deal and you are right as to maybe not show it in the web documentation as it is more of a dev rpc command.

Rename rune to rune_restrictions

done

Add rune_required argument

done, it's now clnrest-register-path path rpc_method [rune_required] [rune_restrictions]

• Should rune_required: False be allowed only when method_type == GET, and default to True for all other methods?

Difficult to say, maybe get some feedback from people who are going to use this?

Add HTTP method types

• Valid values: GET, POST, PUT, PATCH, DELETE

• Default: POST

• Benefits:

  • Semantic clarity - intent is immediately obvious

  • RESTful design - enables resource-oriented API patterns

  • Safety guarantees - GET operations are read-only and idempotent

  • Caching optimization - intermediaries can safely cache GET responses

  • Path reusability - a single path can support multiple operations:

    • GET /user/123 -> Retrieve user details
    • POST /user/123 -> Create a new user
    • PUT /user/123 -> Update an existing user
    • DELETE /user/123 -> Remove the user

We could do this for dynamic routes, but there is a problem: How do you pass on the http method to the rpc method? A http_method argument or do devs have to define separate rpc_method's per "http method+path" combo? With this PR we accept any HTTP method but they all just pass on rpc params from the path and from the request body to the same rpc method.

Content negotiation headers

I suggest explicitly supporting content negotiation headers for completeness, but we can handle this as a separate enhancement in a future PR too.

If you think explicit declarations are preferred and clnrest should reject supported but not declared content types i can easily do that.

• Requests with Accept: text/plain, text/html, or application/x-www-form-urlencoded return 406 Not Acceptable

That is expected since we currently don't support text/plain and text/html. I assume all rpc methods will return json, so what would plain or html look like? Just pretty-print json? application/x-www-form-urlencoded is usually only used in Content-Type. I can't imagine using it for the response body?

• When the Accept header is missing, responses default to a JSON string, requiring additional client-side parsing for text, HTML, or form-encoded use cases

That is not that unusual i think, some REST servers don't even accept Accept headers. The server dictates the format, so this can be expected from the client imo.

[ShahanaFarooqui]

Right, i was just thinking about other plugins. They need to deal with a suddenly appearing/disappearing clnrest if it stays "dynamic" and we don't remember their dynamic routes.

[Shahana]: After discussing with @rustyrussell, we agreed to leave this as-is for now. While there are solutions to address this issue, we'll implement them if a real-world use case requires it.

You are talking about the clnrest-register-path.json file, correct? I don't mind either way, if it's not as a json schema users of cln-rpc won't get a typed call. No that big deal of a deal and you are right as to maybe not show it in the web documentation as it is more of a dev rpc command.

[Shahana]: Per @rustyrussell 's feedback, clnrest-register-path.json should be included in the list (so your current location is correct). However, we should explicitly state in the description that this is a low-level command intended for plugin developers.

Additional recommendations:

• Adding doc/clnrest-register-path.7 \ to the Makefile list https://github.com/ElementsProject/lightning/blob/b15f386df66b31b55e50bb55195bbf835ad1c4c8/doc/Makefile#L38 is necessary for generating its corresponding manpage and .md file.
• I also recommend documenting this feature either in the plugin development guide at https://docs.corelightning.org/docs/plugin-development or adding information about path registration under https://docs.corelightning.org/docs/a-day-in-the-life-of-a-plugin#the-init-method.
• It would be helpful to add 1-2 examples in the JSON schema.

Difficult to say, maybe get some feedback from people who are going to use this?

[Shahana]: Let's keep rune_required: False for GET requests only. POST, PUT, PATCH, and DELETE should always require a rune.

We could do this for dynamic routes, but there is a problem: How do you pass on the http method to the rpc method? A http_method argument or do devs have to define separate rpc_method's per "http method+path" combo? With this PR we accept any HTTP method but they all just pass on rpc params from the path and from the request body to the same rpc method.

[Shahana]: I would prefer a separate rpc_method for each "http_method+path" combination as a cleaner approach. We have two options for that:

1. Pass arrays in arguments in a single clnrest-register-path call: clnrest-register-path path rpc_method|rpc_methods [rune_required|runes_required] [rune_restrictions|runes_restrictions] [http_method|http_methods]

2. Keep the current syntax and let developers call separate clnrest-register-path RPC for different HTTP methods (I prefer this option): clnrest-register-path path rpc_method [rune_required] [rune_restrictions] [http_method]

If you think explicit declarations are preferred and clnrest should reject supported but not declared content types i can easily do that.

[Shahana]: I don't expect explicit declarations for all content types (the default JSON response handling is perfect). However, when clients explicitly declare common content types in headers (like text/plain or text/html), and the plugin developer is prepared to handle and respond in those formats, we shouldn't return a 406 Not Acceptable error. This feature would allow clients to consume responses directly in their expected format. For example, the first request below works correctly, but others fail with errors:

# This works - defaults to JSON
dynamic_res = http_session.post(
	base_url + "/return/json", 
	headers={"Rune": rune}, 
	data={'data': 'Returns JSON'}, 
	verify=ca_cert
)
dynamic_res.raise_for_status()
assert dynamic_res.json()["data"] == "Returns JSON"

# Throws 500 Server Error (should work - explicit JSON content-type)
dynamic_res = http_session.post(
	base_url + "/return/json", 
	headers={"Rune": rune, 'Content-type': 'application/json'}, 
	data={'data': 'Returns JSON'}, 
	verify=ca_cert
)
dynamic_res.raise_for_status()
assert dynamic_res.json()["data"] == "Returns JSON"

# Throws 406 Client Error
dynamic_res = http_session.post(
	base_url + "/return/text", 
	headers={"Rune": rune, 'Content-type': 'text/plain'}, 
	data="I am returning text", 
	verify=ca_cert
)
dynamic_res.raise_for_status()
assert dynamic_res.text == "I am returning text"

# Throws 406 Client Error
dynamic_res = http_session.post(
	base_url + "/return/html", 
	headers={"Rune": rune, 'Content-type': 'text/html'}, 
	data="<h1>I am returning HTML</h1>", 
	verify=ca_cert
)
dynamic_res.raise_for_status()
assert dynamic_res.text == "<h1>I am returning HTML</h1>"

The server should accept these common content types and pass the raw content to the plugin method as a parameter, allowing the plugin developer to handle it appropriately and return the response in the format the client expects.

That is expected since we currently don't support text/plain and text/html. I assume all rpc methods will return json, so what would plain or html look like? Just pretty-print json?

[Shahana]: If we can easily provide this feature, why limit content format support to just JSON? If supporting common formats like text/plain and text/html is straightforward, it could add some flexibility for developers.

application/x-www-form-urlencoded is usually only used in Content-Type. I can't imagine using it for the response body?

[Shahana]: Yeah, this one was specifically for Content-Type. My bad for lumping them together 😛.

That is not that unusual i think, some REST servers don't even accept Accept headers. The server dictates the format, so this can be expected from the client imo.

[Shahana]: If this can be implemented easily, it is good to have. But, if the implementation is complex, the current setup is reasonable too.

Thank you for working through all this feedback! The PR is looking great. ALMOST THERE!!! 👏👏👏

[daywalker90]

It's now clnrest-register-path path rpc_method [http_method] [rune_required] [rune_restrictions] and i think i did everything you asked for except for the content-type stuff.

# Throws 500 Server Error (should work - explicit JSON content-type)
dynamic_res = http_session.post(
	base_url + "/return/json", 
	headers={"Rune": rune, 'Content-type': 'application/json'}, 
	data={'data': 'Returns JSON'}, 
	verify=ca_cert
)
dynamic_res.raise_for_status()
assert dynamic_res.json()["data"] == "Returns JSON"

We have a passing test for this, you can't do it like that: When you pass a dict to data=, requests sends it as form-encoded data (application/x-www-form-urlencoded), which conflicts with your header. You must use json.dumps or json= instead.

I have a commit ready to support any Accept. But Content-Type is more difficult since i build a dictionary for the rpc params and i don't have a key for plain text. I made a dictionary because i gather params from both capture URL's and request body and it would be unclear for an array in which order the elements should be. So either i need a fixed key or a fixed order which everyone has to adhere to.

[ShahanaFarooqui]

We have a passing test for this, you can't do it like that: When you pass a dict to data=, requests sends it as form-encoded data (application/x-www-form-urlencoded), which conflicts with your header. You must use json.dumps or json= instead.

[Shahana]: Ah, that’s a perfect excuse for not testing while sleepy 😄. As long as it works, we are good! 🚀

I have a commit ready to support any Accept. But Content-Type is more difficult since i build a dictionary for the rpc params and i don't have a key for plain text. I made a dictionary because i gather params from both capture URL's and request body and it would be unclear for an array in which order the elements should be.

[Shahana]: Let’s keep it as is for now and address this if a future requirement arises.