Fix nix log with CA derivations

[thufschmitt]

Fix #6209

When trying to run nix log <installable> on a local-fs-store (or indirectly on a
daemon or ssh-ng store), try first to resolve the derivation pointed to
by <installable> as it is the resolved one that holds the build log.

This has a couple of shortcomings:

1. It’s expensive as it requires re-reading the derivation
2. It’s brittle because if the derivation doesn’t exist anymore or can’t
   be resolved (which is the case if any one of its build inputs is missing),
   then we can’t access the log anymore

However, I don’t think we can do better (at least not right now).
The alternatives I see are:

1. Copy the build log for the un-resolved derivation. But that means a
   lot of duplication
2. Store the results of the resolving in the db. Which might be the best
   long-term solution, but leads to a whole new class of potential
   issues.

[Ericson2314]

I recall you had rigged this up so that we got a CA or no CA by default mkDerivation by default?

[Ericson2314]

just as we have some foo and fooUncached store methods, might we have getBuildLogExact and getBuildLogResolved? This logic here isn't specific to this store implementation.

[Ericson2314]

In fact the !path.isDerivation() part is also duplicated between the two stores that implement this, too. That could also go in getBuildLogResolved (or whatever the wrapper function should be called).

[thufschmitt]

I actually did this intentionnally, because as-it-is, the logic assumes that the derivation is there (we can’t resolve without reading it), so that would break with most binary caches

[Ericson2314]

I see, but can't we just see if the derivation exists and only try resolving it if it does?

[Ericson2314]

This inspired me to split out a LogStore class, I am doing that in a separate PR.

[Ericson2314]

Did that in #6220. That will help reduce the clutter of a getBuildLogExact vs getBuildLogResolved split.

I also noticed the daemon protocol has wopAddBuildLog but no wopGetBuildLog. It might be good for security to do the getBuildLogResolved part client-side, and just the getBuildLogExact server side, lest someone try to fish for build logs they shouldn't be able to read with CA output paths. (This information leak attack would work for fixed or floating CA derivations.)

[Ericson2314]

Anyways, I do agree resolving is the right approach. Binary caches should have the trust mappings even if they don't have the drv files for derivations. This means resolving a derivation should not, in fact, require reading the derivation in the happy path (but we can fix that part later).

[Ericson2314]

Thanks! It looks better now, but I would still do

std::optional<std::string> LogStore::getBuildLog(const StorePath & path)
{
    auto maybePath = getBuildDerivationPath(path);
    if (!maybePath)
        return std::nullopt;
    return getBuildLogExact(maybePath.value())
}

where getBuildLog is non-virtual, and getBuildLogExact is virtual = 0.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-26/18252/1

[L-as]

Is this still relevant?

[thufschmitt]

Yes, still relevant. The linked issue is still not fixed

[Ericson2314]

What do you think about my getBuildLog vs getBuildLogExact idea above in #6215 (comment), @thufschmitt?

[Ericson2314]

I would like to separate the concerns of finding the drv vs finding the log. This relates to my hoping we can get rid of --derivation because foo^bar syntax will allow us to be explicit about these things. But I hope it is good even without that.

[thufschmitt]

What do you think about my getBuildLog vs getBuildLogExact idea above in #6215 (comment), @thufschmitt?

Not much, will need to find the time to look into it again 😛

[Ericson2314]

#7430 was a rebase of this that was merged.