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

Add a clue for editors under "sops edit" #1675

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rsrchboy
Copy link

Sometimes there are things we'd prefer our editors do differently when editing files with secrets in them: different configuration, disable certain plugins, etc. This change sets a clue in the launched editor's environment that can be used to easily and trivially determine if the editor was launched by sops edit.

Sometimes there are things we'd prefer our editors do differently when
editing files with secrets in them: different configuration, disable
certain plugins, etc.  This change sets a clue in the launched editor's
environment that can be used to easily and trivially determine if the
editor was launched by `sops edit`.

Signed-off-by: Chris Weyl <[email protected]>
@felixfontein
Copy link
Contributor

Thanks for your contribution! I'm not sure that's a good idea though (are there other programs which do similar things? I'm not aware of any, but that doesn't mean there isn't). I think setting SOPS_EDITOR that will be introduced by #1611 is a better and more general solution.

@rsrchboy
Copy link
Author

rsrchboy commented Nov 18, 2024

Correct me if I'm wrong, but the SOPS_EDITOR variable introduced in #1611 is set by the user, not sops itself, right? If that's the case, there's two problems for trying to use it in this PR's use case:

  1. We cannot depend on SOPS_EDITOR always being set for editors invoked by sops, and
  2. We cannot depend that SOPS_EDITOR being set means we're actually being run under sops, as it is the sort of thing likely to be set in one's ~/.bashrc / ~/.zshrc / etc.

SOPS_EDITOR in #1611 is not a solution to the problem being addressed by this PR: providing an unambiguous, reliable clue that we're in an editor launched by sops edit. These two PRs solve different problems.

@felixfontein
Copy link
Contributor

Well, other programs can also set SOPS_IS_EDITING to true, so I don't see how the solution from this PR is better than the solution from the other PR for this problem.

@rsrchboy
Copy link
Author

Well, other programs can also set SOPS_IS_EDITING to true, so I don't see how the solution from this PR is better than the solution from the other PR for this problem.

Because the work in the other PR is not a solution to this problem.

Nothing we do here can control other programs setting env variables. What we can control is ensuring a specific var will always be set in the editor's environment, for editors launched by sops edit. AFAICT #1611 does not do that with any var.

@felixfontein
Copy link
Contributor

You use #1611 by pointing SOPS_EDITOR to a script that sets whatever env variable you need and then launches your editor. Then you have what this PR does in a way more general form.

Which editor is used and how that editor is configured is highly system dependent (and thus perfectly suited for system-wide or user-wide env variables). If you can control the use of SOPS_IS_EDITING, you can also control setting SOPS_EDITOR.

@rsrchboy
Copy link
Author

You use #1611 by pointing SOPS_EDITOR to a script that sets whatever env variable you need and then launches your editor. Then you have what this PR does in a way more general form.

Ah, I see what you're saying now. Using SOPS_EDITOR to work around this problem is certainly feasible, but doesn't feel quite right, in a way I'm going to have to think about in order to be able to articulate. (Likely that with this PR I'd be depending on something I know sops would set and could just handle it entirely in my .vimrc.)

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

Successfully merging this pull request may close these issues.

2 participants