LNC Sessions: Give more granular control to LND Account Macaroons#1282
LNC Sessions: Give more granular control to LND Account Macaroons#1282Liongrass wants to merge 3 commits into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the flexibility of LNC session creation by allowing LND Account binding to be applied to any session type, rather than being limited to a specific 'account' type. This change enables users to generate more specialized macaroons, such as invoice-only sessions, while maintaining strict account-level restrictions. The implementation refactors how session caveats and permissions are processed to ensure consistent behavior during both initial creation and session resumption. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors session management to allow binding any session type to a specific account using an optional account ID, rather than restricting this functionality to a dedicated 'account' session type. Key changes include updating the CLI flags, refactoring the AddSession RPC to handle account caveats globally, and ensuring resumeSession applies stored caveats for all session types. Feedback suggests that removing the 'account' type from the CLI is a breaking change and recommends retaining it as a preset. Additionally, the resumption logic for custom sessions should be tightened to fail if the required macaroon recipe is missing.
| "determine the permissions a user has when " + | ||
| "connecting with the session; options " + | ||
| "include readonly|admin|account|custom.", | ||
| "include readonly|admin|custom.", |
There was a problem hiding this comment.
Removing the account type from the CLI is a breaking change for users who rely on this preset. While the new --account_id flag allows for more granular control, the account type still serves as a useful preset for the standard set of permissions defined in accounts.MacaroonPermissions. It is recommended to keep it as an option.
include readonly|admin|account|custom.",| case "custom": | ||
| return litrpc.SessionType_TYPE_MACAROON_CUSTOM, nil |
There was a problem hiding this comment.
Since the account session type is still supported by the RPC server and provides a specific set of permissions, it should remain available in the CLI parser to avoid breaking existing scripts and to provide a convenient preset for users.
| case "custom": | |
| return litrpc.SessionType_TYPE_MACAROON_CUSTOM, nil | |
| case "account": | |
| return litrpc.SessionType_TYPE_MACAROON_ACCOUNT, nil | |
| case "custom": | |
| return litrpc.SessionType_TYPE_MACAROON_CUSTOM, nil |
| if sess.MacaroonRecipe != nil { | ||
| permissions = sess.MacaroonRecipe.Permissions | ||
| } |
There was a problem hiding this comment.
For TypeMacaroonCustom and TypeAutopilot, the session's permissions are stored within the MacaroonRecipe. If the recipe is missing, the session is in an invalid state and should fail to resume, similar to how TypeMacaroonAccount is handled above. Proceeding with empty permissions would result in a session that cannot perform any actions.
| if sess.MacaroonRecipe != nil { | |
| permissions = sess.MacaroonRecipe.Permissions | |
| } | |
| if sess.MacaroonRecipe == nil { | |
| return fmt.Errorf("invalid session, expected " + | |
| "recipe to be set") | |
| } | |
| permissions = sess.MacaroonRecipe.Permissions |
|
@Liongrass, remember to re-request review from reviewers when ready |
Disclaimer: This pull request was entirely made with Claude, but tested manually in great detail
Introduction
The command
litcli sessions addallows a user to generate an LNC pairing phrase. This allows a user to permission an external application, for example Zeus on their phone, Alby in the browser, Lnget in the command line or a Wasm like Lightning Terminal. Currently, these sessions can be of--type readonly|admin|account|customAn LND Account is a macaroon with an associated balance. The holder of that macaroon can send and receive, as well as see basic information of the node they are connected to. They cannot open or close channels , and they can only spend funds allocated to their account. Paid invoices created with the Account macaroon are counted towards the Account budget.
Problem
In the current setup, an admin can only create a full LND Account session that can both receive and spend the funds of an LND Account.
While it is possible to create
readonlyorcustommacaroons for an LND Account (see guide here), it is not possible to create an LNC session for it. Most commonly, a user might want to generate an invoice macaroon for their account.Solution
In this pull request, the flag
--account_idno longer requires--type account. Instead,--account_idattenuates any macaroon type defined in--typewithlnd-custom account. This allows the creation of LNC sessions forreadonlyandcustommacaroons restricted to a single LND Account.Testing
I am running this pull request on signet, the created sessions have the expected macaroons with the expected attenuation.
Future Work
Through this change, users can generate LNC sessions for LND Accounts with arbitrary macaroons. This makes new macaroon types easier to create in the future with LND Accounts, such as invoice-only LNC sessions. In the future, the
litcli sessions addcommand could also accept any arbitrary attenuation.Considerations
The macaroon created with the existing
--type account --account_id <account id>differs from that created with--type admin --account_id <account id. While it contains all the admin permissions such asmacaroon:generate, LND doesn't accept these actions because of theaccount_idattenuation.While it would be possible to adjust the LND Account macaroon to be identical to the status quo, the restrictions of an LND Account macaroon are defined by what
litdcan handle, not what is specified in the macaroon. The main disadvantage is that applications like Terminal will currently use the heuristics of the present permissions, rather than thelnd-custom accountattenuation.LND Account Macaroon permissions today:
LND Account "Admin" Macaroon created through the code of this pull request: