Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 105 additions & 22 deletions snap/pack/pack.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"io"
"os"
"path/filepath"
"sort"
"strings"

"github.com/snapcore/snapd/gadget"
Expand Down Expand Up @@ -94,24 +95,23 @@ func debArchitecture(info *snap.Info) string {
}
}

// isDir checks whether relPath exists and is a directory inside the snap
// container.
func isDir(container snap.Container, relPath string) bool {
fi, err := container.Lstat(relPath)
return err == nil && fi.IsDir()
}

// isRegularFile checks whether relPath exists and is a regular file inside the
// snap container.
func isRegularFile(container snap.Container, relPath string) bool {
fi, err := container.Lstat(relPath)
return err == nil && fi.Mode().IsRegular()
}

// validateContentPlugTargets checks that content interface plug target
// directories exist in the snap source tree. This check only applies to
// snaps with base core26 or later.
// directories exist in the snap directory tree.
func validateContentPlugTargets(container snap.Container, info *snap.Info) error {
// An empty base is equivalent to "core".
base := info.Base
if base == "" {
base = "core"
}
// Content plug target validation does not apply to bases
// before core26.
excluded := []string{
"core", "core18", "core20", "core22", "core24",
}
if strutil.ListContains(excluded, base) {
return nil
}

for plugName, plug := range info.Plugs {
if plug.Interface != "content" {
continue
Expand All @@ -138,14 +138,79 @@ func validateContentPlugTargets(container snap.Container, info *snap.Info) error
// only the $SNAP and/or / combination prefix was present
continue
}
fi, err := container.Lstat(relPath)
if err != nil || !fi.IsDir() {
if !isDir(container, relPath) {
return fmt.Errorf("content interface plug %q target %v must exist and must be a directory, ensure it is present in the snap or created before packing", plugName, target)
}
}
return nil
}

// validateLayoutPaths verifies that layout paths located under $SNAP, which are
// used for bind, bind-file as well as a tmpfs target, are present in the snap's
// directory tree.
func validateLayoutPaths(container snap.Container, info *snap.Info) error {
// Sort layout paths for deterministic error reporting.
layoutPaths := make([]string, 0, len(info.Layout))
for p := range info.Layout {
layoutPaths = append(layoutPaths, p)
}
sort.Strings(layoutPaths)

for _, layoutPath := range layoutPaths {
layout := info.Layout[layoutPath]

if layout.Type == "tmpfs" {
// For tmpfs layouts under $SNAP, the mount target directory must
// exist in the snap directory tree to avoid needlessly creating a
// writable mimic at runtime.
if !strings.HasPrefix(layoutPath, "$SNAP") ||
strings.HasPrefix(layoutPath, "$SNAP_DATA") ||
strings.HasPrefix(layoutPath, "$SNAP_COMMON") {
continue
}
relPath := strings.TrimPrefix(layoutPath, "$SNAP")
relPath = strings.TrimPrefix(relPath, "/")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe: we could make a trimSnapPrefix(p) helper that also consider SNAP_DATA and SNAP_COMMON and returns "" in those cases too?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be clear if we do the helper that might not be the best name

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll open a followup PR

if relPath == "" {
continue
}
if !isDir(container, relPath) {
return fmt.Errorf("layout %q must exist as a directory in the snap, ensure it is present or created before packing", layoutPath)
}
continue
}

// TODO: validate symlink targets within $SNAP

// Determine the source path. Only check bind and bind-file. Entries of
// type "tmpfs" were already checked.
source := layout.Bind
if source == "" {
source = layout.BindFile
}
if source == "" {
continue
}

// Only $SNAP paths can be checked at pack time.
if strings.HasPrefix(source, "$SNAP_DATA") || strings.HasPrefix(source, "$SNAP_COMMON") {
continue
}
relPath := strings.TrimPrefix(source, "$SNAP")
relPath = strings.TrimPrefix(relPath, "/")
if relPath == "" {
continue
}

if layout.Bind != "" && !isDir(container, relPath) {
return fmt.Errorf("layout %q source %q must exist and be a directory, ensure it is present in the snap or created before packing", layoutPath, source)
}
if layout.BindFile != "" && !isRegularFile(container, relPath) {
return fmt.Errorf("layout %q source %q must exist and be a file, ensure it is present in the snap or created before packing", layoutPath, source)
}
}
return nil
}

// CheckSkeleton attempts to validate snap data in source directory
func CheckSkeleton(w io.Writer, sourceDir string) error {
yaml, err := os.ReadFile(filepath.Join(sourceDir, "meta", "snap.yaml"))
Expand All @@ -162,6 +227,20 @@ func CheckSkeleton(w io.Writer, sourceDir string) error {
return err
}

func needsStrictLayoutOrContentValidation(info *snap.Info) bool {
// An empty base is equivalent to "core".
base := info.Base
if base == "" {
base = "core"
}
// Strict content plug target or layout paths validation does not apply to
// bases before core26, with the exception of the 'bare' base.
excluded := []string{
"core", "core18", "core20", "core22", "core24",
}
return !strutil.ListContains(excluded, base)
Comment thread
bboozzoo marked this conversation as resolved.
}

func loadAndValidate(sourceDir string, yaml []byte) (*snap.Info, error) {
container := snapdir.New(sourceDir)

Expand All @@ -178,11 +257,15 @@ func loadAndValidate(sourceDir string, yaml []byte) (*snap.Info, error) {
if err := snap.ValidateSnapContainer(container, info, logger.Noticef); err != nil {
return nil, err
}
if err := validateContentPlugTargets(container, info); err != nil {
return nil, err

if needsStrictLayoutOrContentValidation(info) {
if err := validateContentPlugTargets(container, info); err != nil {
return nil, err
}
if err := validateLayoutPaths(container, info); err != nil {
return nil, err
}
}
// TODO: validate content interface slot source (read/write) paths
// exist in the snap source tree, see validateContentPlugTargets.

if _, err := snap.ReadSnapshotYamlFromSnapFile(container); err != nil {
return nil, err
Expand Down
142 changes: 142 additions & 0 deletions snap/pack/pack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -790,3 +790,145 @@ plugs:
err := pack.CheckSkeleton(&buf, sourceDir)
c.Assert(err, ErrorMatches, `content interface plug "plug-missing" target \$SNAP/missing must exist and must be a directory, ensure it is present in the snap or created before packing`)
}

type layoutSourceTestCase struct {
summary string
base string
layout string // layout fragment (indented, under "layout:" key)
plugs string // plugs fragment (indented, under "plugs:" key)
create []string // paths to create: trailing "/" = dir, 'A -> B' = symlink A pointing to B, otherwise a file
errMatch string // expected error regex, "" = no error expected
}

func (s *packSuite) checkSkeletonLayoutPath(c *C, tc layoutSourceTestCase) {
c.Logf("tc: %+v", tc)

yamlStr := fmt.Sprintf("name: hello\nversion: 0\nbase: %s\n", tc.base)
if tc.plugs != "" {
yamlStr += "plugs:\n" + tc.plugs
}
yamlStr += "layout:\n" + tc.layout

sourceDir := makeExampleSnapSourceDir(c, yamlStr)
for _, f := range tc.create {
if before, after, ok := strings.Cut(f, " -> "); ok {
// "name -> target" creates a symlink
path := filepath.Join(sourceDir, before)
c.Assert(os.MkdirAll(filepath.Dir(path), 0755), IsNil)
c.Assert(os.Symlink(after, path), IsNil)
} else if strings.HasSuffix(f, "/") {
path := filepath.Join(sourceDir, f)
c.Assert(os.MkdirAll(path, 0755), IsNil)
} else {
path := filepath.Join(sourceDir, f)
c.Assert(os.MkdirAll(filepath.Dir(path), 0755), IsNil)
c.Assert(os.WriteFile(path, []byte(""), 0644), IsNil)
}
}
var buf bytes.Buffer
err := pack.CheckSkeleton(&buf, sourceDir)
if tc.errMatch == "" {
c.Assert(err, IsNil, Commentf("test: %s", tc.summary))
} else {
c.Assert(err, ErrorMatches, tc.errMatch, Commentf("test: %s", tc.summary))
}
}

func (s *packSuite) TestCheckSkeletonLayoutSourceValid(c *C) {
for _, tc := range []layoutSourceTestCase{
{
summary: "bind source directory exists",
base: "core26",
layout: " /opt/lib:\n bind: $SNAP/lib\n",
create: []string{"lib/"},
}, {
summary: "bind-file source file exists",
base: "core26",
layout: " /opt/foo.conf:\n bind-file: $SNAP/foo.conf\n",
create: []string{"foo.conf"},
}, {
// setup similar to what snapcraft desktop extension injects during build
summary: "source under content target with full path present",
base: "core26",
plugs: " gnome:\n interface: content\n target: $SNAP/gnome-platform\n",
layout: " /usr/lib/webkit:\n bind: $SNAP/gnome-platform/usr/lib/webkit\n",
create: []string{"gnome-platform/usr/lib/webkit/"},
}, {
// symlink target in $SNAP is not required to exist
summary: "symlink layout not checked",
base: "core26",
layout: " /opt/data:\n symlink: $SNAP/data\n",
}, {
summary: "$SNAP_DATA source skipped",
base: "core26",
layout: " /opt/lib:\n bind: $SNAP_DATA/lib\n",
}, {
summary: "$SNAP_COMMON source skipped",
base: "core26",
layout: " /opt/lib:\n bind: $SNAP_COMMON/lib\n",
}, {
summary: "tmpfs under $SNAP with directory present",
base: "core26",
layout: " $SNAP/tmpdir:\n type: tmpfs\n",
create: []string{"tmpdir/"},
}, {
summary: "tmpfs at system path not checked",
base: "core26",
layout: " /usr/share/foo:\n type: tmpfs\n",
},
} {
s.checkSkeletonLayoutPath(c, tc)
}
}

func (s *packSuite) TestCheckSkeletonLayoutSourceInvalid(c *C) {
for _, tc := range []layoutSourceTestCase{
{
summary: "bind source missing",
base: "core26",
layout: " /opt/lib:\n bind: $SNAP/lib\n",
errMatch: `layout "/opt/lib" source "\$SNAP/lib" must exist and be a directory, ensure it is present in the snap or created before packing`,
}, {
summary: "bind source is a file not directory",
base: "core26",
layout: " /opt/lib:\n bind: $SNAP/lib\n",
create: []string{"lib"},
errMatch: `layout "/opt/lib" source "\$SNAP/lib" must exist and be a directory, ensure it is present in the snap or created before packing`,
}, {
summary: "bind-file source is a directory not file",
base: "core26",
layout: " /opt/foo.conf:\n bind-file: $SNAP/foo.conf\n",
create: []string{"foo.conf/"},
errMatch: `layout "/opt/foo.conf" source "\$SNAP/foo.conf" must exist and be a file, ensure it is present in the snap or created before packing`,
}, {
summary: "bind-file source is a symlink not file",
base: "core26",
layout: " /opt/foo.conf:\n bind-file: $SNAP/foo.conf\n",
create: []string{"foo.conf -> some-target"},
errMatch: `layout "/opt/foo.conf" source "\$SNAP/foo.conf" must exist and be a file, ensure it is present in the snap or created before packing`,
}, {
summary: "tmpfs under $SNAP directory missing",
base: "core26",
layout: " $SNAP/missing:\n type: tmpfs\n",
errMatch: `layout "\$SNAP/missing" must exist as a directory in the snap, ensure it is present or created before packing`,
}, {
summary: "tmpfs under $SNAP target is a file not directory",
base: "core26",
layout: " $SNAP/notadir:\n type: tmpfs\n",
create: []string{"notadir"},
errMatch: `layout "\$SNAP/notadir" must exist as a directory in the snap, ensure it is present or created before packing`,
},
} {
s.checkSkeletonLayoutPath(c, tc)
}
}

func (s *packSuite) TestCheckSkeletonLayoutSourceOldBaseSkipped(c *C) {
for _, base := range []string{"core", "core18", "core20", "core22", "core24"} {
s.checkSkeletonLayoutPath(c, layoutSourceTestCase{
summary: "old base " + base,
base: base,
layout: " /opt/lib:\n bind: $SNAP/lib\n",
})
}
Comment thread
bboozzoo marked this conversation as resolved.
}
Loading