Disallow reading flake.lock

[edolstra]

With --no-write-lock-file, it's possible that flake.lock is out of sync with the actual inputs used by the evaluation. So doing fromJSON (readFile ./flake.lock) will give wrong results.

Fixes #4639.

[grahamc]

I’m not sure this is a great idea. 1. It seems to universally block reading flake.lock whether or not it is during a flake evaluation. 2. Also it is possible and not totally unreasonable to imagine a scenario where a nix expressions is reading a file that happens to be named flake.lock which is not the flake.lock which is out of date. 3. One side effect of this change is I think your flake compat wrapper won’t work anymore: <https://github.com/edolstra/flake- compat/blob/12c64ca55c1014cdc1b16ed5a804aa8576601ff2/default.nix#L14> 4. It doesn’t prevent the original bug if someone is trying to read a file which is a symlink to the flake.lock.

On September 14, 2021, GitHub ***@***.***> wrote: Merged #5250 <#5250> into master.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#5250 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe- auth/AAASXLAYJNGP37DW4G7MBVTUB6V2FANCNFSM5EA2GOXA>. Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification- email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification- email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[edolstra]

Yes, this is imperfect. It would probably be better to filter flake.lock while copying the source tree to the store.

3. is a good point, I guess we should allow access to flake.lock in the legacy case.

[thufschmitt]

It would probably be better to filter flake.lock while copying the source tree to the store.

I think that might have some unattended side-effects. For example I have something along the lines of nix.registry.p.flake = ./. in my NixOS config, meaning that I want the flake.lock to be in the store.

3. is a good point, I guess we should allow access to flake.lock in the legacy case.

What about, as a first approximation, only disallow access to the flake.lock when

1. The evaluation runs in “pure” mode, and
2. The flake.lock is at the root of a store path (/nix/store/foobar/flake.lock)
   ?

[grahamc]

I also wonder about synthesizing the correct flake.lock and substituting it in the source tree, then no paths need to be disallowed. 

On September 14, 2021, GitHub ***@***.***> wrote: > It would probably be better to filter flake.lock while copying the > source tree to the store. > I think that might have some unattended side-effects. For example I have something along the lines of nix.registry.p.flake = ./. in my NixOS config, meaning that I want the flake.lock to be in the store. > 1. is a good point, I guess we should allow access to flake.lock in > the legacy case. > What about, as a first approximation, only disallow access to the flake.lock when 1. The evaluation runs in “pure” mode, and 2. The flake.lock is at the root of a store path (/nix/store/foobar/flake.lock)  ?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <#5250 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe- auth/AAASXLAVPNG4WWCUZRYOSNTUCBFLBANCNFSM5EA2GOXA>. Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification- email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification- email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[edolstra]

Yes, the correct solution would be to make a copy of the source tree and write the lock file. That's pretty slow/disk-space-inefficient though...

Alternatively we can mark the top-level source tree as dirty (so no rev etc. attributes are passed). That way configurations that assert a clean tree would at least fail.

[grahamc]

That might be good. One issue with disallowing access to top level flake.locks in pure mode is I think that will break users of the flake compat library for hydra users. 

On September 15, 2021, GitHub ***@***.***> wrote: Yes, the correct solution would be to make a copy of the source tree and write the lock file. That's pretty slow/disk-space-inefficient though... Alternatively we can mark the top-level source tree as dirty (so no rev etc. attributes are passed). That way configurations that assert a clean tree would at least fail.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub <#5250 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe- auth/AAASXLCSKNVDJWUQW236ILLUCB3ARANCNFSM5EA2GOXA>. Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification- email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification- email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[figsoda]

this should only be enabled when using flakes, or else stuff like flake-compat wouldn't work

[tomberek]

fromJSON (readFile ./flake.lock)

Would it be acceptable to expose the in-memory lockFileStr? A small modification to call-flake.nix would do it. This would remove the incentive or need to do the imperfect fromJSON/readFile.

Alternative would be to expose it as a builtin, but that would make it ambiguous with getFlake or in other situations.

eg: tomberek@a6a1e5c