Skip to content

Commit

Permalink
cue/interpreter/embed: respect module boundaries
Browse files Browse the repository at this point in the history
The embed proposal specifies that it should not be possible
to embed files across module boundaries. This CL
implements that restriction.

Also fix in passing an unnecessarily broad "is parent directory"
check: it's OK for a file to start with two dots.

Fixes #3273.

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I4e80edcc49f5d6475ccdd3f46d206e46be55723b
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1197383
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
  • Loading branch information
rogpeppe committed Jul 8, 2024
1 parent ddfe8eb commit 6056916
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 27 deletions.
37 changes: 12 additions & 25 deletions cmd/cue/cmd/testdata/script/embed.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,17 @@ special: {
// These are all valid.
underscoreFile: _ @embed(file="y/_test.json")
dotFile: _ @embed(file="y/.test.json")
dotdotFile: _ @embed(file="..dotdot.json")
underscoreDir: _ @embed(file="_y/test.json")
dotDir: _ @embed(file=".y/test.json")

// TODO: fix nested modules are currently not supported, but we may opt to
// support them in the future. This is currently not handled. It should
// probably be up to the loader to provide a fs.FS that handles this
// according to spec.
nestedModJSON: _ @embed(file="a/b/foo.json")
}

-- test.json --
{ "x": 34 }
-- input.yaml --
a1: 2

-- ..dotdot.json --
{"dotdot": true}
-- y/test.json --
{ "x": 34 }
-- y/_test.json --
Expand Down Expand Up @@ -121,9 +117,9 @@ g: _
special: {
underscoreFile: _
dotFile: _
dotdotFile: _
underscoreDir: _
dotDir: _
nestedModJSON: _
}
-- out/eval --
a: {
Expand Down Expand Up @@ -208,16 +204,15 @@ special: {
dotFile: {
z: 46
}
dotdotFile: {
dotdot: true
}
underscoreDir: {
z: 47
}
dotDir: {
z: 48
}
nestedModJSON: {
a: 1
b: 2
}
}
-- out/export --
a: x: 34
Expand Down Expand Up @@ -265,18 +260,10 @@ g: {
}
special: {
// These are all valid.
underscoreFile: z: 45
dotFile: z: 46
underscoreDir: z: 47
dotDir: z: 48

// TODO: fix nested modules are currently not supported, but we may opt to
// support them in the future. This is currently not handled. It should
// probably be up to the loader to provide a fs.FS that handles this
// according to spec.
nestedModJSON: {
a: 1
b: 2
}
underscoreFile: z: 45
dotFile: z: 46
dotdotFile: dotdot: true
underscoreDir: z: 47
dotDir: z: 48
}
-- out/vet --
12 changes: 12 additions & 0 deletions cmd/cue/cmd/testdata/script/embed_err.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ unspecifiedFiletype2: _ @embed(glob=x/*)
unspecifiedFiletype3: _ @embed(glob=x/*.?son)
unspecifiedFiletype4: _ @embed(glob=x/*.[j]son)

nestedModJSON: _ @embed(file="a/b/foo.json")
nestedModJSONWithGlob: _ @embed(glob="a/*/*.json")

-- test.json --
{ "x": 34 }
-- test.ldjson --
Expand All @@ -80,6 +83,11 @@ a: 1
x: 5
-- xcue --
x: 5
-- a/b/cue.mod/modules.cue --
module: "acme.com"
language: version: "v0.9.0"
-- a/b/foo.json --
{"a": 1, "b": 2}
-- out/std --
-- out/err --
@embed: attribute must have file or glob field:
Expand Down Expand Up @@ -130,3 +138,7 @@ x: 5
./test.cue:49:25
@embed: extension not fully specified; type argument required:
./test.cue:50:25
@embed: cannot embed file "a/b/foo.json": in different module:
./test.cue:52:18
@embed: cannot embed file "a/b/foo.json": in different module:
./test.cue:53:26
23 changes: 21 additions & 2 deletions cue/interpreter/embed/embed.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@ func (c *compiler) processFile(file, scope string, schema adt.Value) (adt.Expr,
if err != nil {
return nil, err
}
for dir := path.Dir(file); dir != "."; dir = path.Dir(dir) {
if _, err := c.fs.Stat(path.Join(dir, "cue.mod")); err == nil {
return nil, errors.Newf(c.pos, "cannot embed file %q: in different module", file)
}
}

return c.decodeFile(file, scope, schema)
}
Expand Down Expand Up @@ -233,6 +238,7 @@ func (c *compiler) processGlob(glob, scope string, schema adt.Value) (adt.Expr,
return nil, errors.Promote(err, "failed to match glob")
}

dirs := make(map[string]string)
for _, f := range matches {
// TODO: lots of stat calls happening in this MVP so another won't hurt.
// We don't support '**' initially, and '*' only matches files, so skip
Expand All @@ -242,6 +248,13 @@ func (c *compiler) processGlob(glob, scope string, schema adt.Value) (adt.Expr,
} else if fi.IsDir() {
continue
}
// Add all parents of the embedded file that
// aren't the current directory (if there's a cue.mod
// in the current directory, that's the current module
// not nested).
for dir := path.Dir(f); dir != "."; dir = path.Dir(dir) {
dirs[dir] = f
}

expr, err := c.decodeFile(f, scope, schema)
if err != nil {
Expand All @@ -253,7 +266,13 @@ func (c *compiler) processGlob(glob, scope string, schema adt.Value) (adt.Expr,
Value: expr,
})
}

// Check that none of the matches were in a nested module
// directory.
for dir, f := range dirs {
if _, err := c.fs.Stat(path.Join(dir, "cue.mod")); err == nil {
return nil, errors.Newf(c.pos, "cannot embed file %q: in different module", f)
}
}
return m, nil
}

Expand All @@ -265,7 +284,7 @@ func (c *compiler) clean(s string) (string, errors.Error) {
if path.IsAbs(file) {
return "", errors.Newf(c.pos, "only relative files are allowed")
}
if strings.HasPrefix(file, "..") {
if file == ".." || strings.HasPrefix(file, "../") {
return "", errors.Newf(c.pos, "cannot refer to parent directory")
}
return file, nil
Expand Down

0 comments on commit 6056916

Please sign in to comment.