feat(helix-loader): allow managing trust from configuration#15602
feat(helix-loader): allow managing trust from configuration#15602maribu wants to merge 4 commits intohelix-editor:masterfrom
Conversation
In the workspace trust code, the code to search a path in a file was copy-pasted three times. This splits it out into a helper function.
|
I'm a little sceptical about the amount of features here. Do we need / want full globbing support? Maybe it's enough to parse the suffix |
I opened #15607 to collect the requirements voiced so far. There are alternative approaches (like using regexes and negative lookahead instead of globbing) to implement an "trust all git worktrees in folder X except for those external branches with following naming pattern". IMO using the
I do generally agree that this is a separate concern. However, being able to prevent the pop ups for good without blindly trusting all work spaces is a hard requirement for some users (I cannot express how much I'm annoyed by pop ups - and others will certainly be, too). And since we need the negation anyway, why not just solve two issues for the cost of one? |
2b982cd to
18d66c0
Compare
I agree there needs to be some way to default-untrust all workspaces to avoid the popups entirely. I don't agree that this solves two issues for the cost of one, though. Stuffing extra syntax into a string value doesn't keep the cost constant. In the below example, I find version (2) to be simpler. # (1)
trust-paths = ["!/**", "~/my-trusted-repos/**"]
# (2)
default-trust = "no" # instead of "prompt"
trusted-paths = ["~/my-trusted-repos/**"] |
|
That is simpler, but it cannot be used to express:
|
|
True. I question whether that complexity is needed. I think your example represents a git worktree where an external PR (untrusted) is checked out. That could be covered by putting all worktrees either in
I understand there's a tradeoff here between more complexity in Helix and asking users to adjust their workflows to compensate for the lacking flexibility of Helix. But adding features later is easier than removing them, so I suggest adding features conservatively. |
|
Just to be sure: Have looked at the code already? It's like three lines of code needed to implement the negation. I'd argue calling that complexity is a bit of a misnomer. |
When the workspace trust files become large, using a BufReader can increase performance, as it won't block until the whole file is loaded into RAM.
…RKSPACES Since `quick_query_workspace()` and `quick_query_workspace_with_explicit_untrust()` are currently called seven times on startup, it makes sense to cache the result for the currently used workspace. With this in place, the previous cache for prompted workspaces could be merged into this. Co-authored-by: xe_nul <iamxenul@gmail.com>
This allows managing trust via the `config.toml` via the `paths` vector in the `[editor.trust]` section. The path of the workspace is matched against the entries in the `path` array while expanding globbing (a `*` matches a single unspecified path component, a `**` matches all subdirectories) and supporting a leading `~/` as shorthand for the home directory. Prefixing a path with `!` can be used to deny trust. The last matching entry wins. For the following example: ```toml [editor.trust] paths = [ "~/repos/helix", "~/repos/foo/*", "~/repos/bar/**", "!~/repos/bar/untrusted" ] ``` This would result in the following trust levels assuming the home directory `/home/user`: | Path | Decision | |:----------------------------------------------- |:------------------- | | `/home/user/foobar` | undecided | | `/home/user/repos/helix` | trusted | | `/home/other/repos/helix` | undecided | | `/home/user/repos/helix/branch_a` | undecided | | `/home/user/repos/foo` | undecided | | `/home/user/repos/foo/branch_a` | trusted | | `/home/user/repos/foo/remote_a/branch_a` | undecided | | `/home/user/repos/bar/branch_a` | trusted | | `/home/user/repos/bar/remote_a/branch_a` | trusted | | `/home/user/repos/bar/untrusted` | untrusted | In case the configuration does not decide to trust or untrust a given working directory, the existing trust mechanism to prompt the user for each new working directory and storing the decision in an internal database is used. In other words: Users that don't want to make use of the new interface and only use the old model can just leave the `config.toml` unchanged and will never be bothered with this. On the other hand users only wanting to use the configuration can do so using: ```toml [editor.trust] paths = [ "!**", "~/repos/helix", ... ] ``` Since the first item (`!**`) matches every work space and denies trust, this configuration will deny trust to all workspaces unless later items override this. The `insecure` configuration from the editor has been removed, as the same result can be achieved with: ```toml [editor.trust] paths= [ "**" ] ```
be4caf5 to
da77e64
Compare
Note
This depends on #15590 and includes all its commits. Only the last commit is added on top of #15590
This allows managing trust via the
config.tomlvia thepathsvector in the[editor.trust]section. The path of the workspace is matched against the entries in thepatharray while expanding globbing (a*matches a single unspecified path component, a**matches allsubdirectories) and supporting a leading
~/as shorthand for the home directory. Prefixing a path with!can be used to deny trust. The last matching entry wins.For the following example:
This would result in the following trust levels assuming the home directory
/home/user:/home/user/foobar/home/user/repos/helix/home/other/repos/helix/home/user/repos/helix/branch_a/home/user/repos/foo/home/user/repos/foo/branch_a/home/user/repos/foo/remote_a/branch_a/home/user/repos/bar/branch_a/home/user/repos/bar/remote_a/branch_a/home/user/repos/bar/untrustedIn case the configuration does not decide to trust or untrust a given working directory, the existing trust mechanism to prompt the user for each new working directory and storing the decision in an internal database is used.
In other words: Users that don't want to make use of the new interface and only use the old model can just leave the
config.tomlunchanged and will never be bothered with this. On the other hand users only wanting to use the configuration can do so using:Since the first item (
!**) matches every work space and denies trust, this configuration will deny trust to all workspaces unless later items override this.The
insecureconfiguration from the editor has been removed, as the same result can be achieved with: