Skip to content

Commit

Permalink
snap, snap/pack: disallow pack and install of snapd, base and os with…
Browse files Browse the repository at this point in the history 
… specific configure hooks (canonical#13117)

* disallow pack/install of snapd, base and os snaps with configure hooks

* overlord/snapstate: keep CheckSnap configure hook validation

* snap, snap/pack: allow configure hook for all OS snaps (core, ubuntu-core)

* snap: fixed hook validation and test for OS snap with both default-configure and configure

* snap/pack: improve unit tests
  • Loading branch information
@ernestl
ernestl authored Feb 20, 2025
1 parent 122ae05 commit 699b1b4
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 23 deletions.
77 changes: 59 additions & 18 deletions snap/pack/pack_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,63 @@ apps:
c.Assert(err, ErrorMatches, `snap is unusable due to missing files: path "bin/hello-world" does not exist`)
}

func (s *packSuite) TestPackKernelGadgetOSAppWithConfigureHookHappy(c *C) {
for _, snapType := range []string{"kernel", "gadget", "os", "app"} {
snapYaml := fmt.Sprintf(`name: %[1]s
version: 0
type: %[1]s`, snapType)
sourceDir := makeExampleSnapSourceDir(c, snapYaml)
c.Assert(os.Mkdir(filepath.Join(sourceDir, "meta", "hooks"), 0755), IsNil)
c.Assert(os.WriteFile(filepath.Join(sourceDir, "meta", "hooks", "configure"), []byte("#!/bin/sh"), 0755), IsNil)
_, err := pack.Pack(sourceDir, pack.Defaults)
c.Assert(err, IsNil)
}
}

func (s *packSuite) TestPackKernelGadgetAppWithDefaultConfigureAndConfigureHookHappy(c *C) {
for _, snapType := range []string{"kernel", "gadget", "app"} {
snapYaml := fmt.Sprintf(`name: %[1]s
version: 0
type: %[1]s`, snapType)
sourceDir := makeExampleSnapSourceDir(c, snapYaml)
configureHooks := []string{"default-configure", "configure"}
c.Assert(os.Mkdir(filepath.Join(sourceDir, "meta", "hooks"), 0755), IsNil)
for _, hook := range configureHooks {
c.Assert(os.WriteFile(filepath.Join(sourceDir, "meta", "hooks", hook), []byte("#!/bin/sh"), 0755), IsNil)
}
_, err := pack.Pack(sourceDir, pack.Defaults)
c.Assert(err, IsNil)
}
}

func (s *packSuite) TestPackSnapdBaseWithConfigureHookError(c *C) {
for _, snapType := range []string{"snapd", "base"} {
snapYaml := fmt.Sprintf(`name: %[1]s
version: 0
type: %[1]s`, snapType)
sourceDir := makeExampleSnapSourceDir(c, snapYaml)
c.Assert(os.Mkdir(filepath.Join(sourceDir, "meta", "hooks"), 0755), IsNil)
c.Assert(os.WriteFile(filepath.Join(sourceDir, "meta", "hooks", "configure"), []byte("#!/bin/sh"), 0755), IsNil)
_, err := pack.Pack(sourceDir, pack.Defaults)
c.Check(err, ErrorMatches, fmt.Sprintf(`cannot validate snap %[1]q: cannot specify "configure" hook for %[1]q snap %[1]q`, snapType))
}
}

func (s *packSuite) TestPackSnapdBaseOSWithDefaultConfigureHookError(c *C) {
for _, snapType := range []string{"snapd", "base", "os"} {
snapYaml := fmt.Sprintf(`name: %[1]s
version: 0
type: %[1]s`, snapType)
sourceDir := makeExampleSnapSourceDir(c, snapYaml)
c.Assert(os.Mkdir(filepath.Join(sourceDir, "meta", "hooks"), 0755), IsNil)
c.Assert(os.WriteFile(filepath.Join(sourceDir, "meta", "hooks", "default-configure"), []byte("#!/bin/sh"), 0755), IsNil)
_, err := pack.Pack(sourceDir, pack.Defaults)
// an error due to a prohibited hook for the snap type takes precedence over the
// error for missing a configure hook when default-configure is present
c.Check(err, ErrorMatches, fmt.Sprintf(`cannot validate snap %[1]q: cannot specify "default-configure" hook for %[1]q snap %[1]q`, snapType))
}
}

func (s *packSuite) TestPackDefaultConfigureWithoutConfigureError(c *C) {
sourceDir := makeExampleSnapSourceDir(c, `name: hello
version: 0
Expand All @@ -177,7 +234,7 @@ apps:
c.Assert(os.Mkdir(filepath.Join(sourceDir, "meta", "hooks"), 0755), IsNil)
c.Assert(os.WriteFile(filepath.Join(sourceDir, "meta", "hooks", "default-configure"), []byte("#!/bin/sh"), 0755), IsNil)
_, err := pack.Pack(sourceDir, pack.Defaults)
c.Check(err, ErrorMatches, "cannot validate snap \"hello\": cannot specify \"default-configure\" hook without \"configure\" hook")
c.Check(err, ErrorMatches, `cannot validate snap "hello": cannot specify "default-configure" hook without "configure" hook`)
}

func (s *packSuite) TestPackConfigureHooksPermissionsError(c *C) {
Expand All @@ -193,28 +250,12 @@ apps:
c.Assert(os.WriteFile(filepath.Join(sourceDir, "meta", "hooks", hook), []byte("#!/bin/sh"), 0644), IsNil)
_, err := pack.Pack(sourceDir, pack.Defaults)
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))
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)
}
}

func (s *packSuite) TestPackConfigureHooksHappy(c *C) {
sourceDir := makeExampleSnapSourceDir(c, `name: hello
version: 0
apps:
foo:
command: bin/hello-world
`)
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"), 0755), IsNil)
_, err := pack.Pack(sourceDir, pack.Defaults)
c.Assert(err, IsNil)
}
}

func (s *packSuite) TestPackSnapshotYamlExcludePathError(c *C) {
sourceDir := makeExampleSnapSourceDir(c, `name: hello
version: 0
Expand Down
20 changes: 19 additions & 1 deletion snap/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,24 @@ func validateHooks(info *Info) error {

hasDefaultConfigureHook := info.Hooks["default-configure"] != nil
hasConfigureHook := info.Hooks["configure"] != nil

if info.SnapType == TypeSnapd || info.SnapType == TypeBase || info.SnapType == TypeOS {
var invalidHooks []string
if hasDefaultConfigureHook {
invalidHooks = append(invalidHooks, `"default-configure"`)
}
if hasConfigureHook && info.SnapType != TypeOS {
invalidHooks = append(invalidHooks, `"configure"`)
}
if len(invalidHooks) > 0 {
// The default-configure hook is not supported for snapd, base or OS snaps.
// The configure hook is also not supported for snapd and base snaps. While
// it is not required for OS snaps (core and ubuntu-core), it is tolerated
// to prevent errors due to existing configure hooks.
return fmt.Errorf("cannot specify %s hook for %q snap %q", strings.Join(invalidHooks, " or "), info.Type(), info.InstanceName())
}
}

if hasDefaultConfigureHook && !hasConfigureHook {
return fmt.Errorf(`cannot specify "default-configure" hook without "configure" hook`)
}
Expand Down Expand Up @@ -457,7 +475,7 @@ func ValidateBase(info *Info) error {
// validate that bases do not have base fields
if info.Type() == TypeOS || info.Type() == TypeBase {
if info.Base != "" && info.Base != "none" {
return fmt.Errorf(`cannot have "base" field on %q snap %q`, info.Type(), info.InstanceName())
return fmt.Errorf(`cannot have "base" field with value other than "none" on %q snap %q`, info.Type(), info.InstanceName())
}
}

Expand Down
84 changes: 80 additions & 4 deletions snap/validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ version: 1.0
c.Assert(Validate(info), IsNil)
}

func (s *ValidateSuite) TestIllegalHookName(c *C) {
func (s *ValidateSuite) TestValidateHookName(c *C) {
hookType := NewHookType(regexp.MustCompile(".*"))
restore := MockSupportedHookTypes([]*HookType{hookType})
defer restore()
Expand All @@ -798,7 +798,83 @@ hooks:
c.Check(err, ErrorMatches, `invalid hook name: "123abc"`)
}

func (s *ValidateSuite) TestIllegalHookDefaultConfigureWithoutConfigure(c *C) {
func (s *ValidateSuite) TestValidateHookSnapdBaseOSWithConfigureHooksError(c *C) {
testMap := map[string]struct {
snapType string
hasDflt bool
hasConf bool
allowConf bool
}{
"snapd-both": {"snapd", true, true, false},
"snapd-dflt-only": {"snapd", true, false, false},
"snapd-conf-only": {"snapd", false, true, false},
"base-both": {"base", true, true, false},
"os-both": {"os", true, true, true},
"os-dflt-only": {"os", true, false, false},
}

var snapYaml []byte
for name, test := range testMap {
var dfltDef, confDef string
if test.hasDflt {
dfltDef = "default-configure:"
}
if test.hasConf {
confDef = "configure:"
}
snapYaml = []byte(fmt.Sprintf(`name: %[1]s
version: 1.0
hooks:
%[2]s
%[3]s
type: %[1]s`, test.snapType, dfltDef, confDef))
info, err := InfoFromSnapYaml(snapYaml)
c.Assert(err, IsNil)
err = Validate(info)
var invalidHooks []string
if test.hasDflt {
invalidHooks = append(invalidHooks, `"default-configure"`)
}
if test.hasConf && !test.allowConf {
invalidHooks = append(invalidHooks, `"configure"`)
}
c.Check(err, ErrorMatches, fmt.Sprintf(`cannot specify %s hook for %[2]q snap %[2]q`,
strings.Join(invalidHooks, " or "), test.snapType), Commentf("Failed test: %s", name))
}
}

func (s *ValidateSuite) TestValidateHookKernelGadgetOSAppWithConfigureHookHappy(c *C) {
var snapYaml []byte
for _, snapType := range []string{"kernel", "gadget", "os", "app"} {
snapYaml = []byte(fmt.Sprintf(`name: %[1]s
version: 1.0
hooks:
configure:
type: %[1]s`, snapType))
info, err := InfoFromSnapYaml(snapYaml)
c.Assert(err, IsNil)
err = Validate(info)
c.Assert(err, IsNil)
}
}

func (s *ValidateSuite) TestValidateHookKernelGadgetAppWithDefaultConfigureHookHappy(c *C) {
var snapYaml []byte
for _, snapType := range []string{"kernel", "gadget", "app"} {
snapYaml = []byte(fmt.Sprintf(`name: %[1]s
version: 1.0
hooks:
default-configure:
configure:
type: %[1]s`, snapType))
info, err := InfoFromSnapYaml(snapYaml)
c.Assert(err, IsNil)
err = Validate(info)
c.Assert(err, IsNil)
}
}

func (s *ValidateSuite) TestValidateHookDefaultConfigureWithoutConfigureError(c *C) {
info, err := InfoFromSnapYaml([]byte(`name: foo
version: 1.0
hooks:
Expand Down Expand Up @@ -1672,7 +1748,7 @@ base: bar
c.Assert(err, IsNil)

err = Validate(info)
c.Check(err, ErrorMatches, `cannot have "base" field on "os" snap "foo"`)
c.Check(err, ErrorMatches, `cannot have "base" field with value other than "none" on "os" snap "foo"`)
}

func (s *ValidateSuite) TestValidateOsCanHaveBaseNone(c *C) {
Expand Down Expand Up @@ -1716,7 +1792,7 @@ base: bar
c.Assert(err, IsNil)

err = Validate(info)
c.Check(err, ErrorMatches, `cannot have "base" field on "base" snap "foo"`)
c.Check(err, ErrorMatches, `cannot have "base" field with value other than "none" on "base" snap "foo"`)
}

func (s *ValidateSuite) TestValidateBaseCanHaveBaseNone(c *C) {
Expand Down

0 comments on commit 699b1b4

Please sign in to comment.