diff --git a/cmd/snap/cmd_pack.go b/cmd/snap/cmd_pack.go index 3b7aa5b914a..8b093ad70ee 100644 --- a/cmd/snap/cmd_pack.go +++ b/cmd/snap/cmd_pack.go @@ -110,7 +110,7 @@ func (x *packCmd) Execute([]string) error { if x.CheckSkeleton { err := pack.CheckSkeleton(Stderr, x.Positional.SnapDir) - if err == snap.ErrMissingPaths { + if errors.Is(err, snap.ErrMissingPaths) { return nil } return err diff --git a/cmd/snap/cmd_pack_test.go b/cmd/snap/cmd_pack_test.go index 74a1da9794d..8ae533ef656 100644 --- a/cmd/snap/cmd_pack_test.go +++ b/cmd/snap/cmd_pack_test.go @@ -107,7 +107,8 @@ func (s *SnapSuite) TestPackPacksFailsForMissingPaths(c *check.C) { snapDir := makeSnapDirForPack(c, packSnapYaml) _, err := snaprun.Parser(snaprun.Client()).ParseArgs([]string{"pack", snapDir, snapDir}) - c.Assert(err, check.ErrorMatches, ".* snap is unusable due to missing files") + // needed files/dirs are tracked in map so order of first error is not guaranteed + c.Assert(err, check.ErrorMatches, `.* snap is unusable due to missing files: path "(bin/hello|bin)" does not exist`) } func (s *SnapSuite) TestPackPacksASnap(c *check.C) { diff --git a/snap/container.go b/snap/container.go index 2889a92ce99..6f637ce24ec 100644 --- a/snap/container.go +++ b/snap/container.go @@ -318,7 +318,7 @@ func validateContainer(c Container, needsrx, needsx, needsr, needsf, noskipd map // bad modes are logged instead of being returned because the end user // can do nothing with the info (and the developer can read the logs) - hasBadModes := false + var firstBadModeErr error err := c.Walk(".", func(path string, info os.FileInfo, err error) error { if err != nil { return err @@ -342,7 +342,9 @@ func validateContainer(c Container, needsrx, needsx, needsr, needsf, noskipd map symlinkInfo, err := evalAndValidateSymlink(c, path) if err != nil { logf("%s", err) - hasBadModes = true + if firstBadModeErr == nil { + firstBadModeErr = err + } } else { // use target mode for checks below mode = symlinkInfo.targetMode @@ -351,33 +353,48 @@ func validateContainer(c Container, needsrx, needsx, needsr, needsf, noskipd map if mode.IsDir() { if mode.Perm()&0555 != 0555 { - logf("in %s %q: %q should be world-readable and executable, and isn't: %s", contType, name, path, mode) - hasBadModes = true + err := fmt.Errorf("%q should be world-readable and executable, and isn't: %s", path, mode) + logf("in %s %q: %v", contType, name, err) + if firstBadModeErr == nil { + firstBadModeErr = err + } } } else { if needsrx[path] { if mode.Perm()&0555 != 0555 { - logf("in snap %q: %q should be world-readable and executable, and isn't: %s", name, path, mode) - hasBadModes = true + err := fmt.Errorf("%q should be world-readable and executable, and isn't: %s", path, mode) + logf("in snap %q: %v", name, err) + if firstBadModeErr == nil { + firstBadModeErr = err + } } } // XXX: do we need to match other directories? if needsf[path] || strings.HasPrefix(path, "meta/") { if mode&(os.ModeNamedPipe|os.ModeSocket|os.ModeDevice) != 0 { - logf("in %s %q: %q should be a regular file (or a symlink) and isn't", contType, name, path) - hasBadModes = true + err := fmt.Errorf("%q should be a regular file (or a symlink) and isn't", path) + logf("in %s %q: ", contType, name, err) + if firstBadModeErr == nil { + firstBadModeErr = err + } } } if needsx[path] || strings.HasPrefix(path, "meta/hooks/") { if mode.Perm()&0111 == 0 { - logf("in %s %q: %q should be executable, and isn't: %s", contType, name, path, mode) - hasBadModes = true + err := fmt.Errorf("%q should be executable, and isn't: %s", path, mode) + logf("in %s %q: %v", contType, name, err) + if firstBadModeErr == nil { + firstBadModeErr = err + } } } else { // in needsr, or under meta but not a hook if mode.Perm()&0444 != 0444 { - logf("in %s %q: %q should be world-readable, and isn't: %s", contType, name, path, mode) - hasBadModes = true + err := fmt.Errorf("%q should be world-readable, and isn't: %s", path, mode) + logf("in %s %q: %v", contType, name, err) + if firstBadModeErr == nil { + firstBadModeErr = err + } } } } @@ -387,18 +404,25 @@ func validateContainer(c Container, needsrx, needsx, needsr, needsf, noskipd map return err } if len(seen) != len(needsx)+len(needsrx)+len(needsr) { + var firstPath string for _, needs := range []map[string]bool{needsx, needsrx, needsr} { for path := range needs { if !seen[path] { logf("in %s %q: path %q does not exist", contType, name, path) + if firstPath == "" { + firstPath = path + } } } } - return ErrMissingPaths + // TODO: aggregate path errors not just the first one + return fmt.Errorf("%w: path %q does not exist", ErrMissingPaths, firstPath) } - if hasBadModes { - return ErrBadModes + if firstBadModeErr != nil { + // TODO: fmt.Errorf("%w: %w", ErrBadModes, firstBadModeErr) when using go 1.20+ + // TODO: aggregate bad mode errors not just the first one + return fmt.Errorf("%w: %v", ErrBadModes, firstBadModeErr) } return nil } diff --git a/snap/container_test.go b/snap/container_test.go index 18588e465b0..febe4de095d 100644 --- a/snap/container_test.go +++ b/snap/container_test.go @@ -84,10 +84,12 @@ version: 1 c.Assert(err, IsNil) err = snap.ValidateSnapContainer(container, info, discard) - c.Check(err, Equals, snap.ErrMissingPaths) + c.Check(err, testutil.ErrorIs, snap.ErrMissingPaths) + c.Check(err, ErrorMatches, `snap is unusable due to missing files: path "meta" does not exist`) err = snap.ValidateComponentContainer(container, "empty-snap+comp.comp", discard) - c.Check(err, Equals, snap.ErrMissingPaths) + c.Check(err, testutil.ErrorIs, snap.ErrMissingPaths) + c.Check(err, ErrorMatches, `snap is unusable due to missing files: path "meta" does not exist`) } func (s *validateSuite) TestValidateContainerEmptyButBadPermFails(c *C) { @@ -107,7 +109,8 @@ version: 1 c.Assert(err, IsNil) err = snap.ValidateSnapContainer(s.container(), info, discard) - c.Check(err, Equals, snap.ErrBadModes) + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, `snap is unusable due to bad permissions: "." should be world-readable and executable, and isn't: drwx------`) } func (s *validateSuite) TestValidateComponentContainerEmptyButBadPermFails(c *C) { @@ -121,16 +124,17 @@ func (s *validateSuite) TestValidateComponentContainerEmptyButBadPermFails(c *C) // snapdir has /meta/component.yaml, but / is 0700 err = snap.ValidateComponentContainer(s.container(), "empty-snap+comp.comp", discard) - c.Check(err, Equals, snap.ErrBadModes) + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, `snap is unusable due to bad permissions: "." should be world-readable and executable, and isn't: drwx------`) } func (s *validateSuite) TestValidateContainerMissingSnapYamlFails(c *C) { const yaml = `name: empty-snap version: 1 ` - container := s.container() c.Assert(os.Chmod(s.snapDirPath, 0755), IsNil) c.Assert(os.Mkdir(filepath.Join(s.snapDirPath, "meta"), 0755), IsNil) + container := s.container() // snapdir's / and /meta are 0755 (i.e. OK), but no /meta/snap.yaml @@ -138,12 +142,14 @@ version: 1 c.Assert(err, IsNil) err = snap.ValidateSnapContainer(container, info, discard) - c.Check(err, Equals, snap.ErrMissingPaths) + c.Check(err, testutil.ErrorIs, snap.ErrMissingPaths) + c.Check(err, ErrorMatches, `snap is unusable due to missing files: path "meta/snap.yaml" does not exist`) // component's / and /meta are 0755 (i.e. OK), but no /meta/component.yaml err = snap.ValidateComponentContainer(container, "empty-snap+comp.comp", discard) - c.Check(err, Equals, snap.ErrMissingPaths) + c.Check(err, testutil.ErrorIs, snap.ErrMissingPaths) + c.Check(err, ErrorMatches, `snap is unusable due to missing files: path "meta/component.yaml" does not exist`) } func (s *validateSuite) TestValidateContainerSnapYamlBadPermsFails(c *C) { @@ -161,7 +167,8 @@ version: 1 c.Assert(err, IsNil) err = snap.ValidateSnapContainer(s.container(), info, discard) - c.Check(err, Equals, snap.ErrBadModes) + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, `snap is unusable due to bad permissions: "meta/snap.yaml" should be world-readable, and isn't: ----------`) } func (s *validateSuite) TestValidateComponentContainerSnapYamlBadPermsFails(c *C) { @@ -173,7 +180,8 @@ func (s *validateSuite) TestValidateComponentContainerSnapYamlBadPermsFails(c *C // /meta/component.yaml exists, but isn't readable err := snap.ValidateComponentContainer(s.container(), "empty-snap+comp.comp", discard) - c.Check(err, Equals, snap.ErrBadModes) + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, `snap is unusable due to bad permissions: "meta/component.yaml" should be world-readable, and isn't: ----------`) } func (s *validateSuite) TestValidateContainerSnapYamlNonRegularFails(c *C) { @@ -191,7 +199,8 @@ version: 1 c.Assert(err, IsNil) err = snap.ValidateSnapContainer(s.container(), info, discard) - c.Check(err, Equals, snap.ErrBadModes) + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, `snap is unusable due to bad permissions: "meta/snap.yaml" should be a regular file \(or a symlink\) and isn't`) } // bootstrapEmptyContainer creates a minimal container directory under @@ -233,7 +242,8 @@ apps: c.Assert(err, IsNil) err = snap.ValidateSnapContainer(s.container(), info, discard) - c.Check(err, Equals, snap.ErrMissingPaths) + c.Check(err, testutil.ErrorIs, snap.ErrMissingPaths) + c.Check(err, ErrorMatches, `snap is unusable due to missing files: path "foo" does not exist`) } func (s *validateSuite) TestValidateContainerBadAppPermsFails(c *C) { @@ -252,7 +262,8 @@ apps: c.Assert(err, IsNil) err = snap.ValidateSnapContainer(s.container(), info, discard) - c.Check(err, Equals, snap.ErrBadModes) + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, `snap is unusable due to bad permissions: "foo" should be world-readable and executable, and isn't: -r--r--r--`) } func (s *validateSuite) TestValidateContainerBadAppDirPermsFails(c *C) { @@ -272,7 +283,8 @@ apps: c.Assert(err, IsNil) err = snap.ValidateSnapContainer(s.container(), info, discard) - c.Check(err, Equals, snap.ErrBadModes) + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, `snap is unusable due to bad permissions: "apps" should be world-readable and executable, and isn't: drwx------`) } func (s *validateSuite) TestValidateContainerBadSvcPermsFails(c *C) { @@ -293,7 +305,8 @@ apps: c.Assert(err, IsNil) err = snap.ValidateSnapContainer(s.container(), info, discard) - c.Check(err, Equals, snap.ErrBadModes) + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, `snap is unusable due to bad permissions: "svcs/bar" should be executable, and isn't: ----------`) } func (s *validateSuite) TestValidateContainerCompleterFails(c *C) { @@ -316,7 +329,8 @@ apps: c.Assert(err, IsNil) err = snap.ValidateSnapContainer(s.container(), info, discard) - c.Check(err, Equals, snap.ErrMissingPaths) + c.Check(err, testutil.ErrorIs, snap.ErrMissingPaths) + c.Check(err, ErrorMatches, `snap is unusable due to missing files: path "comp/foo.sh" does not exist`) } func (s *validateSuite) TestValidateContainerBadAppPathOK(c *C) { @@ -404,7 +418,8 @@ version: 1 c.Assert(err, IsNil) err = snap.ValidateSnapContainer(container, info, discard) - c.Check(err, ErrorMatches, "snap is unusable due to bad permissions") + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, `snap is unusable due to bad permissions: "meta/symlink" should be world-readable, and isn't: -rwx--x--x`) } func (s *validateSuite) TestValidateContainerSymlinksMetaBadTargetMode0000(c *C) { @@ -429,7 +444,8 @@ version: 1 c.Assert(err, IsNil) err = snap.ValidateSnapContainer(container, info, discard) - c.Check(err, ErrorMatches, "snap is unusable due to bad permissions") + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, `snap is unusable due to bad permissions: "meta/symlink" should be world-readable, and isn't: ----------`) } func (s *validateSuite) TestValidateContainerMetaExternalAbsSymlinksFails(c *C) { @@ -450,7 +466,8 @@ version: 1 } err = snap.ValidateSnapContainer(s.container(), info, mockLogf) - c.Check(err, Equals, snap.ErrBadModes) + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, `snap is unusable due to bad permissions: external symlink found: meta/gui/icons/snap.empty-snap.png -> /etc/shadow`) } func (s *validateSuite) TestValidateContainerMetaExternalRelativeSymlinksFails(c *C) { @@ -472,7 +489,8 @@ version: 1 } err = snap.ValidateSnapContainer(s.container(), info, mockLogf) - c.Check(err, Equals, snap.ErrBadModes) + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, `snap is unusable due to bad permissions: external symlink found: meta/gui/icons/snap.empty-snap.png -> 1/../../2/../../3/4/../../../../..`) } func (s *validateSuite) TestValidateContainerMetaExternalRelativeSymlinksOk(c *C) { @@ -518,7 +536,8 @@ version: 1 c.Check(metaDirSymlinkErrFound, Equals, true) // the check for missing files precedes check for permission errors, so we // check for it instead. - c.Check(err, Equals, snap.ErrMissingPaths) + c.Check(err, testutil.ErrorIs, snap.ErrMissingPaths) + c.Check(err, ErrorMatches, `snap is unusable due to missing files: path "meta/snap.yaml" does not exist`) } func (s *validateSuite) TestValidateContainerAppsOK(c *C) { @@ -599,7 +618,8 @@ version: 1 } err = snap.ValidateSnapContainer(s.container(), info, mockLogf) - c.Check(err, Equals, snap.ErrBadModes) + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, "snap is unusable due to bad permissions: too many levels of symbolic links") c.Check(loopFound, Equals, true) } diff --git a/snap/pack/pack_test.go b/snap/pack/pack_test.go index 6d75c099f8f..83ac715182b 100644 --- a/snap/pack/pack_test.go +++ b/snap/pack/pack_test.go @@ -167,7 +167,8 @@ apps: `) c.Assert(os.Remove(filepath.Join(sourceDir, "bin", "hello-world")), IsNil) _, err := pack.Pack(sourceDir, pack.Defaults) - c.Assert(err, Equals, snap.ErrMissingPaths) + c.Check(err, testutil.ErrorIs, snap.ErrMissingPaths) + c.Assert(err, ErrorMatches, `snap is unusable due to missing files: path "bin/hello-world" does not exist`) } func (s *packSuite) TestPackDefaultConfigureWithoutConfigureError(c *C) { @@ -193,9 +194,12 @@ apps: c.Assert(os.Mkdir(filepath.Join(sourceDir, "meta", "hooks"), 0755), IsNil) configureHooks := []string{"configure", "default-configure"} for _, hook := range configureHooks { - c.Assert(os.WriteFile(filepath.Join(sourceDir, "meta", "hooks", hook), []byte("#!/bin/sh"), 0666), IsNil) + c.Assert(os.WriteFile(filepath.Join(sourceDir, "meta", "hooks", hook), []byte("#!/bin/sh"), 0644), IsNil) _, err := pack.Pack(sourceDir, pack.Defaults) - c.Check(err, ErrorMatches, "snap is unusable due to bad permissions") + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Check(err, ErrorMatches, fmt.Sprintf("snap is unusable due to bad permissions: \"meta/hooks/%s\" should be executable, and isn't: -rw-r--r--", hook)) + // Fix hook error to catch next hook's error + c.Assert(os.Chmod(filepath.Join(sourceDir, "meta", "hooks", hook), 755), IsNil) } } @@ -246,7 +250,8 @@ apps: ` c.Assert(os.WriteFile(filepath.Join(sourceDir, "meta", "snapshots.yaml"), []byte(invalidSnapshotYaml), 0411), IsNil) _, err := pack.Pack(sourceDir, pack.Defaults) - c.Assert(err, ErrorMatches, "snap is unusable due to bad permissions") + c.Check(err, testutil.ErrorIs, snap.ErrBadModes) + c.Assert(err, ErrorMatches, `snap is unusable due to bad permissions: "meta/snapshots.yaml" should be world-readable, and isn't: -r----x--x`) } func (s *packSuite) TestPackSnapshotYamlHappy(c *C) { @@ -283,7 +288,8 @@ apps: c.Assert(os.Remove(filepath.Join(sourceDir, "bin", "hello-world")), IsNil) err = pack.CheckSkeleton(&buf, sourceDir) - c.Assert(err, Equals, snap.ErrMissingPaths) + c.Check(err, testutil.ErrorIs, snap.ErrMissingPaths) + c.Assert(err, ErrorMatches, `snap is unusable due to missing files: path "bin/hello-world" does not exist`) c.Check(buf.String(), Equals, "") }