bug#67937: 30.0.50; auth-source-pass relies on epa-file being enabled

[J.P.]

Hi Arsen,

Arsen Arsenović <arsen@aarsen.me> writes:

> "J.P." <jp@neverwas.me> writes:
>
>> 1. Would it be possible to leverage the existing interface from
>>    `epa-hook' for decrypting these files? As a dirty example:
>>
>>    (defun my-ensure-epa-file-name-handler (orig &rest args)
>>      (require 'epa-hook)
>>      (defvar epa-file-handler)
>>      (let ((file-name-handler-alist
>>             (cons epa-file-handler file-name-handler-alist)))
>>        (apply orig args)))
>>
>>    (advice-add 'auth-source-pass--read-entry
>>                :around #'my-ensure-epa-file-name-handler)
>>
>>    And if doing something like that (without the advice, obviously),
>>    could we somehow "weaken" the regexp of our fallback member's key so
>>    that `find-file-name-handlers' favors an existing, user-defined
>>    override? Alternatively, would it be too wasteful to first attempt to
>>    match the target file name against the option's current members
>>    before falling back on binding a modified value (or using your
>>    proposed hard-coded solution)? Or, wasteful or not, what about
>>    instead offering a new auth-source-pass option whose value is an
>>    alist of the same type as `file-name-handler-alist' that we use in
>>    place of or concatenate with the existing value at runtime?
>
> I don't think ensuring the epa-hook is added here is preferable when a
> solution that doesn't rely on hooks (one using epg, like in the patch I
> posted) is quite short.  Unless EPA does more than EPG for this (but it
> does not seem to, to my novice eyes).

I think what `epa-hook' does beyond `epg' is offer the option of opting
out of the latter by way of the `file-name-handler-alist' interface. If
that's unwise or unnecessary for some reason, we should probably explain
why, if only for posterity. And in doing so, we should maybe also
account for behavior like that of `epa-file-insert-file-contents', which
lies in the default `f-n-h-a' code path and appears to go out of its way
to provide a different user experience when it comes to error handling.
If such accommodations are unnecessary, perhaps we ought to be somewhat
explicit as to why.

> I'm not sure what you mean by 'hard-coding' here.  These files are
> always gpg files (that's how pass works), and they are always OpenPGP
> encrypted.  The usage of epg-decrypt-file is proposed by the help of
> epa-decrypt-region IIRC.

I meant "hard-coding" in the sense that the original design seems to
allow external code to potentially influence the decryption process, as
mentioned above. But from what you're saying, it sounds like there's no
legitimate use case for doing so. I wasn't privy to the original design
discussions, but it would be nice to know if there was a good reason for
going this route beyond adhering to the age-old best practice of relying
on interfaces rather than implementations. Perhaps it's worth rifling
through the archives for mention of the authors' original motivations
WRT `f-n-h-a', just for good measure?

>> 2. How likely is it that someone actually depends on the perceived
>>    undesirable behavior currently on HEAD? Like, for example, could
>>    someone out there conceivably have a cron-like script that runs
>>    `epa-file-disable' before copying the encrypted secrets from the
>>    result of an `auth-source-search' to Nextcloud or something? If these
>>    weren't passwords, perhaps we could just shrug off such
>>    hypotheticals, but... (just saying).
>
> I do not know, but this dependency is wrong either way, and can confuse
> users and the auth-source cache.

So, it seems like we're saying that protecting an unlikely minority here
should not stand in the way of simplicity and consistency. I can't argue
against that, but I think it's important to be clear about the potential
consequences of such a sacrifice.

> The only reason I noticed this is because *something* (and I have no
> idea what as of yet) somehow unhooks epa-file.  When I noticed that, I
> figured I could use epa-file-disable to provide a simpler reproducer.  I
> didn't actually notice the bug by using epa-file-disable (and I have
> never intentionally ran epa-file-disable).
>
> I'll be tracking that down next, but fixing this first seemed easier.

Tracking that down would be nice, definitely.

Thanks,
J.P.