diff --git a/snap/pack/pack.go b/snap/pack/pack.go index 942d94477b7..801f0791dfc 100644 --- a/snap/pack/pack.go +++ b/snap/pack/pack.go @@ -24,6 +24,7 @@ import ( "io" "os" "path/filepath" + "sort" "strings" "github.com/snapcore/snapd/gadget" @@ -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 @@ -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, "/") + 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")) @@ -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) +} + func loadAndValidate(sourceDir string, yaml []byte) (*snap.Info, error) { container := snapdir.New(sourceDir) @@ -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 diff --git a/snap/pack/pack_test.go b/snap/pack/pack_test.go index 95d7a9a664c..58a56756f39 100644 --- a/snap/pack/pack_test.go +++ b/snap/pack/pack_test.go @@ -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", + }) + } +}