cue/interpreter/embed: embed should not follow symbolic links

[myitcv]

What version of CUE are you using (cue version)?

$ cue version
cue version v0.0.0-20240712164527-719893f23850

go version go1.22.3
      -buildmode exe
       -compiler gc
  DefaultGODEBUG httplaxcontentlength=1,httpmuxgo121=1,tls10server=1,tlsrsakex=1,tlsunsafeekm=1
     CGO_ENABLED 1
          GOARCH arm64
            GOOS linux
             vcs git
    vcs.revision 719893f23850172d224720e6d1257586179ac895
        vcs.time 2024-07-12T16:45:27Z
    vcs.modified false
cue.lang.version v0.10.0

Does this issue reproduce with the latest release?

Yes

What did you do?

# We need symbolic links
[!unix] skip

# Setup
env CUE_EXPERIMENT=embed
cd $WORK/x
exec ln -s ../top_secret

# Check we cannot reference a symbolic link
! exec cue export x.cue
stderr 'top_secret is a symbolic link'

-- x/cue.mod/module.cue --
module: "mod.example"
language: {
    version: "v0.10.0"
}
-- x/x.cue --
@extern(embed)

package x

x: _ @embed(file=top_secret, type=text)
-- top_secret --
This is a secret

What did you expect to see?

Passing test (for some variation of the error message)

What did you see instead?

# We need symbolic links (0.000s)
# Setup (0.001s)
# Check we cannot reference a symbolic link (0.013s)
> ! exec cue export x.cue
[stdout]
{
    "x": "This is a secret\n"
}
FAIL: /tmp/testscript613483604/repro.txtar/script.txtar:10: unexpected command success

[myitcv]

Noting the "conclusion" I proposed in https://github.com/cue-lang/cue/issues/3300#issuecomment-2242088546, I still remain of the opinion that @embed should not read from symlinks, even in the main module. Such behaviour would be consistent with Go, which also does not follow symlinks when it comes to embedded files. This would, however, create an exception to the "rule" proposed in that conclusion.

[rogpeppe]

See the comment here: https://github.com/cue-lang/cue/issues/3300#issuecomment-2260210547

Also, it's not currently trivial to add symlink checks in embed as it uses fs.FS which doesn't make it possible to check for symbolic links in general (see https://github.com/golang/go/issues/49580).

[mvdan]

We have gone back and forth on this in offline discussions, so I'll try to summarize the current state:

1) Per #3300, it turns out that Bazel users also need symlink support for embedding. This is a minor but present problem with Go: https://github.com/golang/go/issues/59924 2) As embedding uses io/fs.FS, and io/fs.SymlinkFS is not merged yet (https://github.com/golang/go/issues/49580), forbidding symlinks is possible only by going around the io/fs abstraction and doing os.Lstat. It works, but it's not ideal. 3) Forbidding embedding via os.Lstat or io/fs.SymlinkFS.StatLink is non-trivial. It works when embedding a symlink file itself, but when the symlink is a directory along a path like glob="symlink-dir/*/*.json", this becomes harder. Neither fs.Open nor fs.Glob treat symlinks any different, so we would have to manually check every directory level leading up to a file too.

We have added test cases for embedding symlinks in https://review.gerrithub.io/c/cue-lang/cue/+/1199032 and https://review.gerrithub.io/c/cue-lang/cue/+/1199041, which capture the current behavior. Those are merged for rc.1.

For the reasons above, https://review.gerrithub.io/c/cue-lang/cue/+/1199041 will not be merged for rc.1. Particularly, it fails on point 3 - it does not forbid glob="symlink-dir/*/*.json". We will leave this issue open to discuss on Monday to decide what to do for the final release.

Assuming that #3300 converges on "we must support symlinks when loading packages and when embedding files for good Bazel compatibility" in the coming days, I'd be fine with shipping v0.10.0 with that implementation.

[mvdan]

I am moving this to v0.11 per the above - we are releasing v0.10.0 this week and it seems like @rogpeppe is confident with leaving symlink support in place in @embed. @rogpeppe please follow up on #3300 and update this issue with your latest conclusion as soon as you're able to.