From 60320babd96149ca0a2c807e93c27fc1f9b73e90 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 26 Nov 2025 11:01:08 +0100 Subject: [PATCH 01/54] feat: search, load and validates manifests Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 81 ++++++++++++++++++++ internal/manifestutil/manifestutil.go | 102 +++++++++++++++++++++++++- 2 files changed, 182 insertions(+), 1 deletion(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 35c81a79..8dedcfb8 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -2,15 +2,21 @@ package main import ( "fmt" + "os" + "path" + "path/filepath" "slices" "time" "github.com/jessevdk/go-flags" + "github.com/canonical/chisel/internal/apacheutil" "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/cache" + "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" + "github.com/canonical/chisel/public/manifest" ) var shortCutHelp = "Cut a tree with selected slices" @@ -73,6 +79,17 @@ func (cmd *cmdCut) Execute(args []string) error { } } + previousSliceKeys, err := recut(release, cmd.RootDir) + if err != nil { + return err + } + // Merge with explicitly requested slice keys + for _, s := range previousSliceKeys { + if !slices.Contains(sliceKeys, s) { + sliceKeys = append(sliceKeys, s) + } + } + selection, err := setup.Select(release, sliceKeys, cmd.Arch) if err != nil { return err @@ -128,3 +145,67 @@ func (cmd *cmdCut) Execute(args []string) error { }) return err } + +func recut(release *setup.Release, rootdir string) ([]setup.SliceKey, error) { + manifests := manifestutil.FindPathsInRelease(release) + + if len(manifests) == 0 { + // TODO: When enabling the feature, error out if no manifest found. + // For now do nothing. + return nil, nil + } + + // Get targetDir path + targetDir := filepath.Clean(rootdir) + if !filepath.IsAbs(targetDir) { + dir, err := os.Getwd() + if err != nil { + return nil, fmt.Errorf("cannot obtain current directory: %w", err) + } + targetDir = filepath.Join(dir, targetDir) + } + + entries, err := os.ReadDir(targetDir) + if err != nil { + return nil, fmt.Errorf("cannot read root directory %q: %v", targetDir, err) + } + + if len(entries) == 0 { + // Empty dir, so cutting a new rootfs + return nil, nil + } + + // Root dir is not empty, cutting on top of an existing rootfs. + + // Select the first manifest of the list as the reference one for now. + // Another heuristic could be used (ex. select the one from base-files_chisel). + firstManifestPath := path.Join(targetDir, manifests[0]) + firstManifest, err := manifestutil.Read(firstManifestPath) + if err != nil { + logf("Warning: Cannot read manifest %q from the root directory: %v", firstManifestPath, err) + return nil, nil + } + + err = manifestutil.CheckManifests(firstManifestPath, targetDir, manifests[1:]) + if err != nil { + // TODO: When enabling the feature, error out. + logf("Warning: %v", err) + return nil, nil + } + + // TODO + // validate given target rootdir with manifest content + + // Get slices used to build the rootfs + sliceKeys := []setup.SliceKey{} + firstManifest.IterateSlices("", func(slice *manifest.Slice) error { + sk, err := apacheutil.ParseSliceKey(slice.Name) + if err != nil { + return err + } + sliceKeys = append(sliceKeys, sk) + return nil + }) + + return sliceKeys, nil +} diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 16b05402..842eb898 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -1,9 +1,12 @@ package manifestutil import ( + "crypto/sha256" "fmt" "io" "io/fs" + "os" + "path" "path/filepath" "slices" "sort" @@ -14,6 +17,7 @@ import ( "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/public/jsonwall" "github.com/canonical/chisel/public/manifest" + "github.com/klauspost/compress/zstd" ) const DefaultFilename = "manifest.wall" @@ -34,6 +38,24 @@ func FindPaths(slices []*setup.Slice) map[string][]*setup.Slice { return manifestSlices } +// FindPathsInRelease finds all the paths marked with "generate:manifest" +// for the given release. +func FindPathsInRelease(r *setup.Release) []string { + manifests := []string{} + for _, pkg := range r.Packages { + for _, slice := range pkg.Slices { + for path, info := range slice.Contents { + if info.Generate == setup.GenerateManifest { + dir := strings.TrimSuffix(path, "**") + path = filepath.Join(dir, DefaultFilename) + manifests = append(manifests, path) + } + } + } + } + return manifests +} + type WriteOptions struct { PackageInfo []*archive.PackageInfo Selection []*setup.Slice @@ -134,7 +156,7 @@ func manifestAddReport(dbw *jsonwall.DBWriter, report *Report) error { func unixPerm(mode fs.FileMode) (perm uint32) { perm = uint32(mode.Perm()) if mode&fs.ModeSticky != 0 { - perm |= 01000 + perm |= 0o1000 } return perm } @@ -340,3 +362,81 @@ func Validate(mfest *manifest.Manifest) (err error) { } return nil } + +// Read reads, validates and returns a manifest. +func Read(manifestPath string) (*manifest.Manifest, error) { + f, err := os.Open(manifestPath) + if err != nil { + return nil, err + } + defer f.Close() + + r, err := zstd.NewReader(f) + if err != nil { + return nil, err + } + defer r.Close() + + mfest, err := manifest.Read(r) + if err != nil { + return nil, err + } + + err = Validate(mfest) + if err != nil { + return nil, err + } + + return mfest, nil +} + +func hash(path string) ([]byte, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return nil, err + } + + return h.Sum(nil), nil +} + +func CheckManifests(reference string, targetDir string, manifests []string) error { + hashReference, err := hash(path.Join(targetDir, reference)) + if err != nil { + return err + } + + infoRef, err := os.Stat(reference) + if err != nil { + return err + } + + modeRef := infoRef.Mode() + + for _, m := range manifests { + infoManifest, err := os.Stat(m) + if err != nil { + return err + } + + modeManifest := infoManifest.Mode() + if modeManifest != modeRef { + return fmt.Errorf("invalid manifest: permissions on %s (%s) are different from the reference manifest %s (%s)", m, modeManifest, reference, modeRef) + } + + hashM, err := hash(m) + if err != nil { + return err + } + if !slices.Equal(hashM, hashReference) { + return fmt.Errorf("invalid manifest: %s is inconsistent with %s", m, reference) + } + } + + return nil +} From c67c66162c271bcceeea37ba236e993b49d319c4 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 26 Nov 2025 11:04:27 +0100 Subject: [PATCH 02/54] fix: revert unintentional change Signed-off-by: Paul Mars --- internal/manifestutil/manifestutil.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 842eb898..8adb5f25 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -156,7 +156,7 @@ func manifestAddReport(dbw *jsonwall.DBWriter, report *Report) error { func unixPerm(mode fs.FileMode) (perm uint32) { perm = uint32(mode.Perm()) if mode&fs.ModeSticky != 0 { - perm |= 0o1000 + perm |= 01000 } return perm } From 22ae256cadbbbe07e5d43d120e8356c1fda92e11 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 26 Nov 2025 11:37:44 +0100 Subject: [PATCH 03/54] refactor: move things around Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 53 +++++++++++---------------- internal/manifestutil/manifestutil.go | 18 ++++++++- 2 files changed, 38 insertions(+), 33 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 8dedcfb8..0f4b7319 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -10,13 +10,11 @@ import ( "github.com/jessevdk/go-flags" - "github.com/canonical/chisel/internal/apacheutil" "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/cache" "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" - "github.com/canonical/chisel/public/manifest" ) var shortCutHelp = "Cut a tree with selected slices" @@ -79,10 +77,12 @@ func (cmd *cmdCut) Execute(args []string) error { } } - previousSliceKeys, err := recut(release, cmd.RootDir) + previousSliceKeys, err := canRecut(release, cmd.RootDir) if err != nil { return err } + // TODO validate rootfs + // Merge with explicitly requested slice keys for _, s := range previousSliceKeys { if !slices.Contains(sliceKeys, s) { @@ -146,16 +146,11 @@ func (cmd *cmdCut) Execute(args []string) error { return err } -func recut(release *setup.Release, rootdir string) ([]setup.SliceKey, error) { - manifests := manifestutil.FindPathsInRelease(release) - - if len(manifests) == 0 { - // TODO: When enabling the feature, error out if no manifest found. - // For now do nothing. - return nil, nil - } - +// canRecut determines if the existing target was produced by chisel. +// If possible extracts the list of slices used to produce it. +func canRecut(release *setup.Release, rootdir string) ([]setup.SliceKey, error) { // Get targetDir path + // Note: This is already done in `slicer.Run`. Extract it and avoid doing it twice? targetDir := filepath.Clean(rootdir) if !filepath.IsAbs(targetDir) { dir, err := os.Getwd() @@ -175,37 +170,31 @@ func recut(release *setup.Release, rootdir string) ([]setup.SliceKey, error) { return nil, nil } - // Root dir is not empty, cutting on top of an existing rootfs. - + manifests := manifestutil.FindPathsInRelease(release) + if len(manifests) == 0 { + // No manifest in the release means it cannot produce a rootfs that can + // be recut. Treat this case as cutting a new rootfs. + return nil, nil + } + // Select the first manifest of the list as the reference one for now. // Another heuristic could be used (ex. select the one from base-files_chisel). - firstManifestPath := path.Join(targetDir, manifests[0]) - firstManifest, err := manifestutil.Read(firstManifestPath) + refManifestPath := path.Join(targetDir, manifests[0]) + refManifest, err := manifestutil.Read(refManifestPath) if err != nil { - logf("Warning: Cannot read manifest %q from the root directory: %v", firstManifestPath, err) + logf("Warning: Cannot read manifest %q from the root directory: %v", refManifestPath, err) return nil, nil } - - err = manifestutil.CheckManifests(firstManifestPath, targetDir, manifests[1:]) + + err = manifestutil.VerifyManifests(refManifestPath, targetDir, manifests[1:]) if err != nil { // TODO: When enabling the feature, error out. logf("Warning: %v", err) return nil, nil } - + // TODO // validate given target rootdir with manifest content - // Get slices used to build the rootfs - sliceKeys := []setup.SliceKey{} - firstManifest.IterateSlices("", func(slice *manifest.Slice) error { - sk, err := apacheutil.ParseSliceKey(slice.Name) - if err != nil { - return err - } - sliceKeys = append(sliceKeys, sk) - return nil - }) - - return sliceKeys, nil + return manifestutil.SliceKeys(refManifest), nil } diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 8adb5f25..353b71fe 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -405,7 +405,9 @@ func hash(path string) ([]byte, error) { return h.Sum(nil), nil } -func CheckManifests(reference string, targetDir string, manifests []string) error { +// VerifyManifests checks consistency between a list of manifests and a +// reference one. +func VerifyManifests(reference string, targetDir string, manifests []string) error { hashReference, err := hash(path.Join(targetDir, reference)) if err != nil { return err @@ -440,3 +442,17 @@ func CheckManifests(reference string, targetDir string, manifests []string) erro return nil } + +func SliceKeys(m *manifest.Manifest) []setup.SliceKey { + sliceKeys := []setup.SliceKey{} + m.IterateSlices("", func(slice *manifest.Slice) error { + sk, err := apacheutil.ParseSliceKey(slice.Name) + if err != nil { + return err + } + sliceKeys = append(sliceKeys, sk) + return nil + }) + + return sliceKeys +} From 6cb437fd9f5d69f8c94ce494c40740653d433e13 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 26 Nov 2025 15:09:20 +0100 Subject: [PATCH 04/54] refactor: move logic to manifestutil Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 62 ++++++++---------------- internal/manifestutil/manifestutil.go | 69 +++++++++++++++++++-------- 2 files changed, 71 insertions(+), 60 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 0f4b7319..0b8617e7 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -3,7 +3,6 @@ package main import ( "fmt" "os" - "path" "path/filepath" "slices" "time" @@ -77,16 +76,25 @@ func (cmd *cmdCut) Execute(args []string) error { } } - previousSliceKeys, err := canRecut(release, cmd.RootDir) + rootfsPath, err := preExistingRootfs(cmd.RootDir) if err != nil { return err } - // TODO validate rootfs - // Merge with explicitly requested slice keys - for _, s := range previousSliceKeys { - if !slices.Contains(sliceKeys, s) { - sliceKeys = append(sliceKeys, s) + if len(rootfsPath) > 0 { + manifest, err := manifestutil.RootFSManifest(release, rootfsPath) + if err != nil { + return err + } + + // Merge the slice keys used to build the existing rootfs with the ones + // explicitly requested slice. + // Previous slice keys contains both explicitly requested slices and slices + // resolved following essentials. + for _, s := range manifestutil.SliceKeys(manifest) { + if !slices.Contains(sliceKeys, s) { + sliceKeys = append(sliceKeys, s) + } } } @@ -146,55 +154,27 @@ func (cmd *cmdCut) Execute(args []string) error { return err } -// canRecut determines if the existing target was produced by chisel. -// If possible extracts the list of slices used to produce it. -func canRecut(release *setup.Release, rootdir string) ([]setup.SliceKey, error) { +// preExistingRootfs determines if the target directory was produced by chisel. +func preExistingRootfs(rootdir string) (string, error) { // Get targetDir path // Note: This is already done in `slicer.Run`. Extract it and avoid doing it twice? targetDir := filepath.Clean(rootdir) if !filepath.IsAbs(targetDir) { dir, err := os.Getwd() if err != nil { - return nil, fmt.Errorf("cannot obtain current directory: %w", err) + return "", fmt.Errorf("cannot obtain current directory: %w", err) } targetDir = filepath.Join(dir, targetDir) } entries, err := os.ReadDir(targetDir) if err != nil { - return nil, fmt.Errorf("cannot read root directory %q: %v", targetDir, err) + return "", fmt.Errorf("cannot read root directory %q: %v", targetDir, err) } if len(entries) == 0 { - // Empty dir, so cutting a new rootfs - return nil, nil - } - - manifests := manifestutil.FindPathsInRelease(release) - if len(manifests) == 0 { - // No manifest in the release means it cannot produce a rootfs that can - // be recut. Treat this case as cutting a new rootfs. - return nil, nil + return "", nil } - // Select the first manifest of the list as the reference one for now. - // Another heuristic could be used (ex. select the one from base-files_chisel). - refManifestPath := path.Join(targetDir, manifests[0]) - refManifest, err := manifestutil.Read(refManifestPath) - if err != nil { - logf("Warning: Cannot read manifest %q from the root directory: %v", refManifestPath, err) - return nil, nil - } - - err = manifestutil.VerifyManifests(refManifestPath, targetDir, manifests[1:]) - if err != nil { - // TODO: When enabling the feature, error out. - logf("Warning: %v", err) - return nil, nil - } - - // TODO - // validate given target rootdir with manifest content - - return manifestutil.SliceKeys(refManifest), nil + return targetDir, nil } diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 353b71fe..9bcd0626 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -363,8 +363,38 @@ func Validate(mfest *manifest.Manifest) (err error) { return nil } -// Read reads, validates and returns a manifest. -func Read(manifestPath string) (*manifest.Manifest, error) { +// RootFSManifest extracts the manifest from a targetDir +func RootFSManifest(release *setup.Release, targetDir string) (*manifest.Manifest, error) { + manifestPaths := FindPathsInRelease(release) + if len(manifestPaths) == 0 { + // No manifest in the release means it cannot produce a rootfs that can + // be recut. Treat this case as cutting a new rootfs. + return nil, nil + } + + // Select the first manifest of the list as the reference one for now. + // Another heuristic could be used (ex. select the one from base-files_chisel). + refManifestPath := path.Join(targetDir, manifestPaths[0]) + refManifest, err := read(refManifestPath) + if err != nil { + logf("Warning: Cannot read manifest %q from the root directory: %v", refManifestPath, err) + return nil, nil + } + + err = checkConsistency(refManifestPath, targetDir, manifestPaths[1:]) + if err != nil { + // TODO: When enabling the feature, error out. + logf("Warning: %v", err) + return nil, nil + } + + // TODO + // validate given target rootdir consistent with the manifest + return refManifest, nil +} + +// read reads, validates and returns a manifest. +func read(manifestPath string) (*manifest.Manifest, error) { f, err := os.Open(manifestPath) if err != nil { return nil, err @@ -390,24 +420,9 @@ func Read(manifestPath string) (*manifest.Manifest, error) { return mfest, nil } -func hash(path string) ([]byte, error) { - f, err := os.Open(path) - if err != nil { - return nil, err - } - defer f.Close() - - h := sha256.New() - if _, err := io.Copy(h, f); err != nil { - return nil, err - } - - return h.Sum(nil), nil -} - -// VerifyManifests checks consistency between a list of manifests and a +// checkConsistency checks consistency between a list of manifests and a // reference one. -func VerifyManifests(reference string, targetDir string, manifests []string) error { +func checkConsistency(reference string, targetDir string, manifests []string) error { hashReference, err := hash(path.Join(targetDir, reference)) if err != nil { return err @@ -443,6 +458,22 @@ func VerifyManifests(reference string, targetDir string, manifests []string) err return nil } +func hash(path string) ([]byte, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return nil, err + } + + return h.Sum(nil), nil +} + +// SliceKeys returns setup.SliceKeys from a manifest. func SliceKeys(m *manifest.Manifest) []setup.SliceKey { sliceKeys := []setup.SliceKey{} m.IterateSlices("", func(slice *manifest.Slice) error { From 00825de80a694b6d7c11d6c259b5aef9868009d3 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 27 Nov 2025 11:43:21 +0100 Subject: [PATCH 05/54] feat: validate rootfs with manifest Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 3 +- internal/manifestutil/manifestutil.go | 90 ++++++++++++++++++++++++--- 2 files changed, 82 insertions(+), 11 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 0b8617e7..b1688eed 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -84,7 +84,8 @@ func (cmd *cmdCut) Execute(args []string) error { if len(rootfsPath) > 0 { manifest, err := manifestutil.RootFSManifest(release, rootfsPath) if err != nil { - return err + // TODO: When enabling the feature, error out. + logf("Warning: %v", err) } // Merge the slice keys used to build the existing rootfs with the ones diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 9bcd0626..f986750a 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -2,6 +2,7 @@ package manifestutil import ( "crypto/sha256" + "encoding/hex" "fmt" "io" "io/fs" @@ -11,6 +12,7 @@ import ( "slices" "sort" "strings" + "syscall" "github.com/canonical/chisel/internal/apacheutil" "github.com/canonical/chisel/internal/archive" @@ -375,26 +377,25 @@ func RootFSManifest(release *setup.Release, targetDir string) (*manifest.Manifes // Select the first manifest of the list as the reference one for now. // Another heuristic could be used (ex. select the one from base-files_chisel). refManifestPath := path.Join(targetDir, manifestPaths[0]) - refManifest, err := read(refManifestPath) + refManifest, err := load(refManifestPath) if err != nil { - logf("Warning: Cannot read manifest %q from the root directory: %v", refManifestPath, err) - return nil, nil + return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", refManifestPath, err) } err = checkConsistency(refManifestPath, targetDir, manifestPaths[1:]) if err != nil { - // TODO: When enabling the feature, error out. - logf("Warning: %v", err) - return nil, nil + return nil, err } - // TODO - // validate given target rootdir consistent with the manifest + err = validateRootfs(refManifest, targetDir) + if err != nil { + return nil, err + } return refManifest, nil } -// read reads, validates and returns a manifest. -func read(manifestPath string) (*manifest.Manifest, error) { +// load reads, validates and returns a manifest. +func load(manifestPath string) (*manifest.Manifest, error) { f, err := os.Open(manifestPath) if err != nil { return nil, err @@ -487,3 +488,72 @@ func SliceKeys(m *manifest.Manifest) []setup.SliceKey { return sliceKeys } + +func validateRootfs(m *manifest.Manifest, rootDir string) error { + return m.IteratePaths("", func(path *manifest.Path) error { + p := filepath.Join(rootDir, path.Path) + info, err := os.Lstat(p) + if err != nil { + return err + } + mode := info.Mode() + if mode.String() != path.Mode { + return fmt.Errorf("tampered content: %q mode mismatch: %s recorded, %s observed", path.Path, path.Mode, mode) + } + + // Verify directories + if strings.HasSuffix(path.Path, "/") { + if !info.IsDir() { + return fmt.Errorf("tampered content: %q expected to be a directory", path.Path) + } + // Nothing more to check + return nil + } + + // Verify symlinks + if len(path.Link) > 0 { + link, err := os.Readlink(p) + if err != nil { + return err + } + + if link != path.Link { + return fmt.Errorf("tampered content: %q link destination mismatch: %s recorded, %s observed", path.Path, path.Link, link) + } + return nil + } + + if !mode.IsRegular() { + return fmt.Errorf("tampered content: unrecognized type of file.") + } + + // Verify hardlinks + if path.Inode != 0 { + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return fmt.Errorf("cannot get syscall stat info for %q", path.Path) + } + nLink := stat.Nlink + if nLink != path.Inode { + return fmt.Errorf("tampered content: %q hardlinkq count mismatch: %s recorded, %s observed", path.Path, path.Inode, nLink) + } + } + + // Common verification for regular files + // TODO skip for manifest.wall + + h, err := hash(p) + if err != nil { + return err + } + hString := hex.EncodeToString(h) + if len(path.FinalSHA256) > 0 { + if hString != path.FinalSHA256 { + return fmt.Errorf("tampered file %q, hash mismatch: %s recorded, %s observed", path.Path, path.FinalSHA256, hString) + } + } else if len(path.SHA256) > 0 && hString != path.SHA256 { + return fmt.Errorf("tampered file %q, sha256 mismatch: %s recorded, %s observed", path.Path, path.SHA256, hString) + } + return nil + }) +} From 442bcc6486e16bc90584212b686ddf06ff5c6fae Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 27 Nov 2025 13:34:32 +0100 Subject: [PATCH 06/54] fix: debugging Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 18 +++++++++--------- internal/manifestutil/manifestutil.go | 19 +++++++++++-------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index b1688eed..3fd8c181 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -86,15 +86,15 @@ func (cmd *cmdCut) Execute(args []string) error { if err != nil { // TODO: When enabling the feature, error out. logf("Warning: %v", err) - } - - // Merge the slice keys used to build the existing rootfs with the ones - // explicitly requested slice. - // Previous slice keys contains both explicitly requested slices and slices - // resolved following essentials. - for _, s := range manifestutil.SliceKeys(manifest) { - if !slices.Contains(sliceKeys, s) { - sliceKeys = append(sliceKeys, s) + } else { + // Merge the slice keys used to build the existing rootfs with the ones + // explicitly requested slice. + // Previous slice keys contains both explicitly requested slices and slices + // resolved following essentials. + for _, s := range manifestutil.SliceKeys(manifest) { + if !slices.Contains(sliceKeys, s) { + sliceKeys = append(sliceKeys, s) + } } } } diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index f986750a..e12f6931 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -424,7 +424,7 @@ func load(manifestPath string) (*manifest.Manifest, error) { // checkConsistency checks consistency between a list of manifests and a // reference one. func checkConsistency(reference string, targetDir string, manifests []string) error { - hashReference, err := hash(path.Join(targetDir, reference)) + hashReference, err := hash(reference) if err != nil { return err } @@ -437,7 +437,7 @@ func checkConsistency(reference string, targetDir string, manifests []string) er modeRef := infoRef.Mode() for _, m := range manifests { - infoManifest, err := os.Stat(m) + infoManifest, err := os.Stat(path.Join(targetDir, m)) if err != nil { return err } @@ -489,6 +489,10 @@ func SliceKeys(m *manifest.Manifest) []setup.SliceKey { return sliceKeys } +// validateRootfs verify the content of the target directory is in line with +// the manifest. +// This function works under the assumption the manifest was previously +// validated. func validateRootfs(m *manifest.Manifest, rootDir string) error { return m.IteratePaths("", func(path *manifest.Path) error { p := filepath.Join(rootDir, path.Path) @@ -497,8 +501,8 @@ func validateRootfs(m *manifest.Manifest, rootDir string) error { return err } mode := info.Mode() - if mode.String() != path.Mode { - return fmt.Errorf("tampered content: %q mode mismatch: %s recorded, %s observed", path.Path, path.Mode, mode) + if fmt.Sprintf("0%o", unixPerm(mode)) != path.Mode { + return fmt.Errorf("tampered content: %q mode mismatch: %s recorded, %s observed", path.Path, path.Mode, mode.String()) } // Verify directories @@ -506,7 +510,6 @@ func validateRootfs(m *manifest.Manifest, rootDir string) error { if !info.IsDir() { return fmt.Errorf("tampered content: %q expected to be a directory", path.Path) } - // Nothing more to check return nil } @@ -535,13 +538,12 @@ func validateRootfs(m *manifest.Manifest, rootDir string) error { } nLink := stat.Nlink if nLink != path.Inode { - return fmt.Errorf("tampered content: %q hardlinkq count mismatch: %s recorded, %s observed", path.Path, path.Inode, nLink) + logf("tampered content: %q hardlinks count mismatch: %d recorded, %d observed", path.Path, path.Inode, nLink) + // return fmt.Errorf("tampered content: %q hardlinks count mismatch: %d recorded, %d observed", path.Path, path.Inode, nLink) } } // Common verification for regular files - // TODO skip for manifest.wall - h, err := hash(p) if err != nil { return err @@ -552,6 +554,7 @@ func validateRootfs(m *manifest.Manifest, rootDir string) error { return fmt.Errorf("tampered file %q, hash mismatch: %s recorded, %s observed", path.Path, path.FinalSHA256, hString) } } else if len(path.SHA256) > 0 && hString != path.SHA256 { + // This is effectively skipping the manifest.wall since no hash is declared on it return fmt.Errorf("tampered file %q, sha256 mismatch: %s recorded, %s observed", path.Path, path.SHA256, hString) } return nil From e19f6f1e02ae83529230f708ab35602a997d60ee Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 27 Nov 2025 14:32:59 +0100 Subject: [PATCH 07/54] fix: improve hardlinks check Signed-off-by: Paul Mars --- internal/manifestutil/manifestutil.go | 38 ++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index e12f6931..a0cf8cbc 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -494,6 +494,11 @@ func SliceKeys(m *manifest.Manifest) []setup.SliceKey { // This function works under the assumption the manifest was previously // validated. func validateRootfs(m *manifest.Manifest, rootDir string) error { + hardLinkGroups, err := groupHardlinks(m) + if err != nil { + return err + } + return m.IteratePaths("", func(path *manifest.Path) error { p := filepath.Join(rootDir, path.Path) info, err := os.Lstat(p) @@ -532,14 +537,21 @@ func validateRootfs(m *manifest.Manifest, rootDir string) error { // Verify hardlinks if path.Inode != 0 { + paths, ok := hardLinkGroups[path.Inode] + if !ok { + return fmt.Errorf("cannot find paths associated to this inode: %d", path.Inode) + } + stat, ok := info.Sys().(*syscall.Stat_t) if !ok { return fmt.Errorf("cannot get syscall stat info for %q", path.Path) } nLink := stat.Nlink - if nLink != path.Inode { - logf("tampered content: %q hardlinks count mismatch: %d recorded, %d observed", path.Path, path.Inode, nLink) - // return fmt.Errorf("tampered content: %q hardlinks count mismatch: %d recorded, %d observed", path.Path, path.Inode, nLink) + + if int(nLink) != len(paths) { + // Working under the assumption no hardlink not managed by chisel + // and pointing at a chisel-managed file was added. + return fmt.Errorf("tampered content: %q hardlinks count mismatch: %d recorded, %d observed", path.Path, len(paths), nLink) } } @@ -560,3 +572,23 @@ func validateRootfs(m *manifest.Manifest, rootDir string) error { return nil }) } + +// groupHardlinks groups hardlink paths by inode. +func groupHardlinks(m *manifest.Manifest) (map[uint64][]*manifest.Path, error) { + hardLinkGroups := make(map[uint64][]*manifest.Path) + + err := m.IteratePaths("", func(path *manifest.Path) error { + inode := path.Inode + if inode == 0 { + return nil + } + hardLinkGroups[inode] = append(hardLinkGroups[inode], path) + + return nil + }) + if err != nil { + return nil, err + } + + return hardLinkGroups, nil +} From b6ecc27577b1a534e1b5d6b70483ab8cf5afa605 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 28 Nov 2025 15:47:46 +0100 Subject: [PATCH 08/54] feat: trying alternative approach Signed-off-by: Paul Mars --- internal/manifestutil/manifestutil.go | 166 +++++++++++++++++++++++++- 1 file changed, 165 insertions(+), 1 deletion(-) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index a0cf8cbc..6c418466 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -11,6 +11,7 @@ import ( "path/filepath" "slices" "sort" + "strconv" "strings" "syscall" @@ -387,10 +388,27 @@ func RootFSManifest(release *setup.Release, targetDir string) (*manifest.Manifes return nil, err } - err = validateRootfs(refManifest, targetDir) + rootfsReport, err := reportFromRootfs(targetDir) if err != nil { return nil, err } + fmt.Printf("Report: %#v\n", rootfsReport) + + manifestReport, err := reportFromManifest(refManifest) + if err != nil { + return nil, err + } + fmt.Printf("Report from manifest: %#v\n", manifestReport) + + err = validateReportEntries(manifestReport, rootfsReport) + if err != nil { + return nil, err + } + + // err = validateRootfs(refManifest, targetDir) + // if err != nil { + // return nil, err + // } return refManifest, nil } @@ -511,6 +529,7 @@ func validateRootfs(m *manifest.Manifest, rootDir string) error { } // Verify directories + // TODO check sha and final_sha and size if strings.HasSuffix(path.Path, "/") { if !info.IsDir() { return fmt.Errorf("tampered content: %q expected to be a directory", path.Path) @@ -592,3 +611,148 @@ func groupHardlinks(m *manifest.Manifest) (map[uint64][]*manifest.Path, error) { return hardLinkGroups, nil } + +// reportFromRootfs builds a Report from root directory +func reportFromRootfs(rootDir string) (*Report, error) { + report, err := NewReport(rootDir) + if err != nil { + return nil, fmt.Errorf("internal error: cannot create report: %w", err) + } + + var inodes []uint64 + pathsByInodes := make(map[uint64][]string) + + dirfs := os.DirFS(report.Root) + err = fs.WalkDir(dirfs, ".", func(path string, d fs.DirEntry, err error) error { + if err != nil { + return fmt.Errorf("walk error: %w", err) + } + if path == "." { + return nil + } + fpath := filepath.Join(report.Root, path) + finfo, err := d.Info() + if err != nil { + return fmt.Errorf("cannot get stat info for %q: %w", fpath, err) + } + + entry := ReportEntry{ + Mode: finfo.Mode(), + } + + var size int + + ftype := finfo.Mode() & fs.ModeType + switch ftype { + case fs.ModeDir: + path = "/" + path + "/" + case fs.ModeSymlink: + lpath, err := os.Readlink(fpath) + if err != nil { + return err + } + path = "/" + path + entry.Link = lpath + case 0: // Regular + data, err := os.ReadFile(fpath) + if err != nil { + return fmt.Errorf("cannot read file: %w", err) + } + if len(data) >= 0 { + sum := sha256.Sum256(data) + entry.SHA256 = hex.EncodeToString(sum[:]) + } + path = "/" + path + size = int(finfo.Size()) + default: + return fmt.Errorf("unknown file type %d: %s", ftype, fpath) + } + entry.Path = path + entry.Size = size + + if ftype != fs.ModeDir { + stat, ok := finfo.Sys().(*syscall.Stat_t) + if !ok { + return fmt.Errorf("cannot get syscall stat info for %q", fpath) + } + inode := stat.Ino + if len(pathsByInodes[inode]) == 1 { + inodes = append(inodes, inode) + } + entry.Inode = inode + pathsByInodes[inode] = append(pathsByInodes[inode], path) + } + + report.Entries[path] = entry + + return nil + }) + + return report, nil +} + +// reportFromManifest builds a Report from a Manifest +func reportFromManifest(m *manifest.Manifest) (*Report, error) { + report := &Report{ + Entries: make(map[string]ReportEntry), + } + + err := m.IteratePaths("", func(path *manifest.Path) error { + mode, err := strconv.ParseUint(path.Mode, 8, 32) + if err != nil { + return fmt.Errorf("cannot parse mode: %v", err) + } + + entry := ReportEntry{ + Path: path.Path, + Mode: fs.FileMode(mode), + Size: int(path.Size), + FinalSHA256: path.FinalSHA256, + SHA256: path.SHA256, + Inode: path.Inode, + Link: path.Link, + } + report.Entries[path.Path] = entry + + return nil + }) + + return report, err +} + +// validateReportEntries validates report entries of toValidate against +// entries in the reference. The report under validation can contain more +// entries. +func validateReportEntries(reference, toValidate *Report) error { + for path, referenceEntry := range reference.Entries { + var errorMessage string + entryToValidate, ok := toValidate.Entries[path] + if !ok { + return fmt.Errorf("%q is missing", path) + } + + if entryToValidate.Mode != referenceEntry.Mode { + errorMessage = fmt.Sprintf("invalid mode: expected %q, found %q", referenceEntry.Mode, entryToValidate.Mode) + } + if entryToValidate.Size != referenceEntry.Size { + errorMessage = fmt.Sprintf("invalid size: expected %d, found %d", referenceEntry.Size, entryToValidate.Size) + } + if len(referenceEntry.FinalSHA256) > 0 { + if entryToValidate.SHA256 != referenceEntry.FinalSHA256 { + errorMessage = fmt.Sprintf("invalid hash: expected %s, found %s", referenceEntry.FinalSHA256, entryToValidate.SHA256) + } + } else if len(referenceEntry.SHA256) > 0 && entryToValidate.SHA256 != referenceEntry.SHA256 { + errorMessage = fmt.Sprintf("invalid hash: expected %s, found %s", referenceEntry.SHA256, entryToValidate.SHA256) + } + + if entryToValidate.Link != referenceEntry.Link { + errorMessage = fmt.Sprintf("invalid link: expected %q, found %q", referenceEntry.Link, entryToValidate.Link) + } + + if len(errorMessage) > 0 { + return fmt.Errorf("invalid entry %q: %s", path, errorMessage) + } + } + + return nil +} From 1406b18e83dd1b8cb1d0a16f59551f03f18302d1 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 1 Dec 2025 13:33:07 +0100 Subject: [PATCH 09/54] feat: refine first approach Signed-off-by: Paul Mars --- internal/manifestutil/manifestutil.go | 244 +++++++++++++++++--------- 1 file changed, 161 insertions(+), 83 deletions(-) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 6c418466..67f84eeb 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -388,27 +388,27 @@ func RootFSManifest(release *setup.Release, targetDir string) (*manifest.Manifes return nil, err } - rootfsReport, err := reportFromRootfs(targetDir) - if err != nil { - return nil, err - } - fmt.Printf("Report: %#v\n", rootfsReport) - - manifestReport, err := reportFromManifest(refManifest) - if err != nil { - return nil, err - } - fmt.Printf("Report from manifest: %#v\n", manifestReport) + // rootfsReport, err := reportFromRootfs(targetDir) + // if err != nil { + // return nil, err + // } + // fmt.Printf("Report: %#v\n", rootfsReport) - err = validateReportEntries(manifestReport, rootfsReport) - if err != nil { - return nil, err - } + // manifestReport, err := reportFromManifest(refManifest) + // if err != nil { + // return nil, err + // } + // fmt.Printf("Report from manifest: %#v\n", manifestReport) - // err = validateRootfs(refManifest, targetDir) + // err = validateReportEntries(manifestReport, rootfsReport) // if err != nil { // return nil, err // } + + err = validateRootfs(refManifest, targetDir) + if err != nil { + return nil, err + } return refManifest, nil } @@ -511,108 +511,186 @@ func SliceKeys(m *manifest.Manifest) []setup.SliceKey { // the manifest. // This function works under the assumption the manifest was previously // validated. +// Files not managed by chisel are not checked. func validateRootfs(m *manifest.Manifest, rootDir string) error { - hardLinkGroups, err := groupHardlinks(m) + pathGroups, err := groupManifestPaths(m) if err != nil { return err } - return m.IteratePaths("", func(path *manifest.Path) error { - p := filepath.Join(rootDir, path.Path) - info, err := os.Lstat(p) + for _, group := range pathGroups { + err := verifyGroup(rootDir, group) if err != nil { return err } - mode := info.Mode() - if fmt.Sprintf("0%o", unixPerm(mode)) != path.Mode { - return fmt.Errorf("tampered content: %q mode mismatch: %s recorded, %s observed", path.Path, path.Mode, mode.String()) - } + } + + return nil +} + +type pathGroup struct { + headEntry *manifest.Path + paths []string +} + +// groupManifestPaths groups paths by inode. +func groupManifestPaths(m *manifest.Manifest) (map[string]*pathGroup, error) { + pathGroups := make(map[string]*pathGroup) + inodeGroups := make(map[uint64]string) - // Verify directories - // TODO check sha and final_sha and size - if strings.HasSuffix(path.Path, "/") { - if !info.IsDir() { - return fmt.Errorf("tampered content: %q expected to be a directory", path.Path) + err := m.IteratePaths("", func(path *manifest.Path) error { + inode := path.Inode + if inode == 0 { + // New single path + pathGroups[path.Path] = &pathGroup{ + headEntry: path, + paths: []string{path.Path}, } return nil } - // Verify symlinks - if len(path.Link) > 0 { - link, err := os.Readlink(p) - if err != nil { - return err - } - - if link != path.Link { - return fmt.Errorf("tampered content: %q link destination mismatch: %s recorded, %s observed", path.Path, path.Link, link) + groupKey, ok := inodeGroups[inode] + if !ok { + // New group of hardlink + pathGroups[path.Path] = &pathGroup{ + headEntry: path, + paths: []string{path.Path}, } + inodeGroups[inode] = path.Path return nil } + // Add path to the existing group + group := pathGroups[groupKey] + group.paths = append(group.paths, path.Path) - if !mode.IsRegular() { - return fmt.Errorf("tampered content: unrecognized type of file.") - } + return nil + }) + if err != nil { + return nil, err + } - // Verify hardlinks - if path.Inode != 0 { - paths, ok := hardLinkGroups[path.Inode] - if !ok { - return fmt.Errorf("cannot find paths associated to this inode: %d", path.Inode) - } + // sort paths + for _, group := range pathGroups { + slices.Sort(group.paths) + } - stat, ok := info.Sys().(*syscall.Stat_t) - if !ok { - return fmt.Errorf("cannot get syscall stat info for %q", path.Path) - } - nLink := stat.Nlink + return pathGroups, nil +} - if int(nLink) != len(paths) { - // Working under the assumption no hardlink not managed by chisel - // and pointing at a chisel-managed file was added. - return fmt.Errorf("tampered content: %q hardlinks count mismatch: %d recorded, %d observed", path.Path, len(paths), nLink) - } +// verifyGroup verifies a group of paths +func verifyGroup(rootDir string, group *pathGroup) error { + path := group.headEntry + + info, err := verifyPath(rootDir, path) + if err != nil { + return err + } + + // Verify hardlinks + if len(group.paths) == 0 { + // Not a hardlink, nothing more to do + return nil + } + + headInode, err := getPhysicalInode(info) + if err != nil { + return err + } + + return verifyHardlinks(headInode, rootDir, path.Path, group.paths) +} + +func verifyPath(rootDir string, path *manifest.Path) (os.FileInfo, error) { + p := filepath.Join(rootDir, path.Path) + info, err := os.Lstat(p) + if err != nil { + return nil, err + } + mode := info.Mode() + if fmt.Sprintf("0%o", unixPerm(mode)) != path.Mode { + return nil, fmt.Errorf("tampered content: %q mode mismatch: %s recorded, %s observed", path.Path, path.Mode, mode.String()) + } + + // Verify directories + if strings.HasSuffix(path.Path, "/") { + if !info.IsDir() { + return nil, fmt.Errorf("tampered content: %q expected to be a directory", path.Path) } + return info, nil + } - // Common verification for regular files - h, err := hash(p) + // Verify symlinks + if len(path.Link) > 0 { + link, err := os.Readlink(p) if err != nil { - return err + return nil, err } - hString := hex.EncodeToString(h) - if len(path.FinalSHA256) > 0 { - if hString != path.FinalSHA256 { - return fmt.Errorf("tampered file %q, hash mismatch: %s recorded, %s observed", path.Path, path.FinalSHA256, hString) - } - } else if len(path.SHA256) > 0 && hString != path.SHA256 { - // This is effectively skipping the manifest.wall since no hash is declared on it - return fmt.Errorf("tampered file %q, sha256 mismatch: %s recorded, %s observed", path.Path, path.SHA256, hString) + + if link != path.Link { + return nil, fmt.Errorf("tampered content: %q link destination mismatch: %s recorded, %s observed", path.Path, path.Link, link) } - return nil - }) -} + return info, nil + } -// groupHardlinks groups hardlink paths by inode. -func groupHardlinks(m *manifest.Manifest) (map[uint64][]*manifest.Path, error) { - hardLinkGroups := make(map[uint64][]*manifest.Path) + if !mode.IsRegular() { + return nil, fmt.Errorf("tampered content: unrecognized type of file.") + } - err := m.IteratePaths("", func(path *manifest.Path) error { - inode := path.Inode - if inode == 0 { - return nil - } - hardLinkGroups[inode] = append(hardLinkGroups[inode], path) + // Verify size + if info.Size() != int64(path.Size) { + return nil, fmt.Errorf("tampered file: %q, size mimsmatch: %d recorded, %d observed", path.Path, info.Size(), int64(path.Size)) + } - return nil - }) + // Common verification for regular files + // Most expensive operation, so do it at the end. + h, err := hash(p) if err != nil { return nil, err } + hString := hex.EncodeToString(h) + if len(path.FinalSHA256) > 0 { + if hString != path.FinalSHA256 { + return nil, fmt.Errorf("tampered file %q, hash mismatch: %s recorded, %s observed", path.Path, path.FinalSHA256, hString) + } + } else if len(path.SHA256) > 0 && hString != path.SHA256 { + // This is effectively skipping the manifest.wall since no hash is declared on it + return nil, fmt.Errorf("tampered file %q, sha256 mismatch: %s recorded, %s observed", path.Path, path.SHA256, hString) + } + return info, nil +} - return hardLinkGroups, nil +func verifyHardlinks(headInode uint64, rootDir string, headPath string, paths []string) error { + for _, siblingPath := range paths[1:] { + siblingFullPath := filepath.Join(rootDir, siblingPath) + + sibFi, err := os.Lstat(siblingFullPath) + if err != nil { + return err + } + + // Verify Inode Equality + sibInode, err := getPhysicalInode(sibFi) + if err != nil { + return err + } + + if sibInode != headInode { + return fmt.Errorf("tampered content: broken hardlink: %s and %s should share inode but do not", headPath, siblingPath) + } + } + return nil +} + +func getPhysicalInode(info os.FileInfo) (uint64, error) { + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return 0, fmt.Errorf("cannot get syscall stat info for %q", info.Name()) + } + return stat.Ino, nil } // reportFromRootfs builds a Report from root directory +// Implementation heavily borrowed from testutil.TreeDump func reportFromRootfs(rootDir string) (*Report, error) { report, err := NewReport(rootDir) if err != nil { From 8aba03540b437e4da0a4ac249bf99e1400e90550 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 1 Dec 2025 13:52:39 +0100 Subject: [PATCH 10/54] refactor: dedicated verify.go Signed-off-by: Paul Mars --- internal/manifestutil/manifestutil.go | 266 -------------------------- internal/manifestutil/verify.go | 194 +++++++++++++++++++ 2 files changed, 194 insertions(+), 266 deletions(-) create mode 100644 internal/manifestutil/verify.go diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 67f84eeb..dacb6320 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -11,7 +11,6 @@ import ( "path/filepath" "slices" "sort" - "strconv" "strings" "syscall" @@ -388,23 +387,6 @@ func RootFSManifest(release *setup.Release, targetDir string) (*manifest.Manifes return nil, err } - // rootfsReport, err := reportFromRootfs(targetDir) - // if err != nil { - // return nil, err - // } - // fmt.Printf("Report: %#v\n", rootfsReport) - - // manifestReport, err := reportFromManifest(refManifest) - // if err != nil { - // return nil, err - // } - // fmt.Printf("Report from manifest: %#v\n", manifestReport) - - // err = validateReportEntries(manifestReport, rootfsReport) - // if err != nil { - // return nil, err - // } - err = validateRootfs(refManifest, targetDir) if err != nil { return nil, err @@ -507,188 +489,6 @@ func SliceKeys(m *manifest.Manifest) []setup.SliceKey { return sliceKeys } -// validateRootfs verify the content of the target directory is in line with -// the manifest. -// This function works under the assumption the manifest was previously -// validated. -// Files not managed by chisel are not checked. -func validateRootfs(m *manifest.Manifest, rootDir string) error { - pathGroups, err := groupManifestPaths(m) - if err != nil { - return err - } - - for _, group := range pathGroups { - err := verifyGroup(rootDir, group) - if err != nil { - return err - } - } - - return nil -} - -type pathGroup struct { - headEntry *manifest.Path - paths []string -} - -// groupManifestPaths groups paths by inode. -func groupManifestPaths(m *manifest.Manifest) (map[string]*pathGroup, error) { - pathGroups := make(map[string]*pathGroup) - inodeGroups := make(map[uint64]string) - - err := m.IteratePaths("", func(path *manifest.Path) error { - inode := path.Inode - if inode == 0 { - // New single path - pathGroups[path.Path] = &pathGroup{ - headEntry: path, - paths: []string{path.Path}, - } - return nil - } - - groupKey, ok := inodeGroups[inode] - if !ok { - // New group of hardlink - pathGroups[path.Path] = &pathGroup{ - headEntry: path, - paths: []string{path.Path}, - } - inodeGroups[inode] = path.Path - return nil - } - // Add path to the existing group - group := pathGroups[groupKey] - group.paths = append(group.paths, path.Path) - - return nil - }) - if err != nil { - return nil, err - } - - // sort paths - for _, group := range pathGroups { - slices.Sort(group.paths) - } - - return pathGroups, nil -} - -// verifyGroup verifies a group of paths -func verifyGroup(rootDir string, group *pathGroup) error { - path := group.headEntry - - info, err := verifyPath(rootDir, path) - if err != nil { - return err - } - - // Verify hardlinks - if len(group.paths) == 0 { - // Not a hardlink, nothing more to do - return nil - } - - headInode, err := getPhysicalInode(info) - if err != nil { - return err - } - - return verifyHardlinks(headInode, rootDir, path.Path, group.paths) -} - -func verifyPath(rootDir string, path *manifest.Path) (os.FileInfo, error) { - p := filepath.Join(rootDir, path.Path) - info, err := os.Lstat(p) - if err != nil { - return nil, err - } - mode := info.Mode() - if fmt.Sprintf("0%o", unixPerm(mode)) != path.Mode { - return nil, fmt.Errorf("tampered content: %q mode mismatch: %s recorded, %s observed", path.Path, path.Mode, mode.String()) - } - - // Verify directories - if strings.HasSuffix(path.Path, "/") { - if !info.IsDir() { - return nil, fmt.Errorf("tampered content: %q expected to be a directory", path.Path) - } - return info, nil - } - - // Verify symlinks - if len(path.Link) > 0 { - link, err := os.Readlink(p) - if err != nil { - return nil, err - } - - if link != path.Link { - return nil, fmt.Errorf("tampered content: %q link destination mismatch: %s recorded, %s observed", path.Path, path.Link, link) - } - return info, nil - } - - if !mode.IsRegular() { - return nil, fmt.Errorf("tampered content: unrecognized type of file.") - } - - // Verify size - if info.Size() != int64(path.Size) { - return nil, fmt.Errorf("tampered file: %q, size mimsmatch: %d recorded, %d observed", path.Path, info.Size(), int64(path.Size)) - } - - // Common verification for regular files - // Most expensive operation, so do it at the end. - h, err := hash(p) - if err != nil { - return nil, err - } - hString := hex.EncodeToString(h) - if len(path.FinalSHA256) > 0 { - if hString != path.FinalSHA256 { - return nil, fmt.Errorf("tampered file %q, hash mismatch: %s recorded, %s observed", path.Path, path.FinalSHA256, hString) - } - } else if len(path.SHA256) > 0 && hString != path.SHA256 { - // This is effectively skipping the manifest.wall since no hash is declared on it - return nil, fmt.Errorf("tampered file %q, sha256 mismatch: %s recorded, %s observed", path.Path, path.SHA256, hString) - } - return info, nil -} - -func verifyHardlinks(headInode uint64, rootDir string, headPath string, paths []string) error { - for _, siblingPath := range paths[1:] { - siblingFullPath := filepath.Join(rootDir, siblingPath) - - sibFi, err := os.Lstat(siblingFullPath) - if err != nil { - return err - } - - // Verify Inode Equality - sibInode, err := getPhysicalInode(sibFi) - if err != nil { - return err - } - - if sibInode != headInode { - return fmt.Errorf("tampered content: broken hardlink: %s and %s should share inode but do not", headPath, siblingPath) - } - } - return nil -} - -func getPhysicalInode(info os.FileInfo) (uint64, error) { - stat, ok := info.Sys().(*syscall.Stat_t) - if !ok { - return 0, fmt.Errorf("cannot get syscall stat info for %q", info.Name()) - } - return stat.Ino, nil -} - // reportFromRootfs builds a Report from root directory // Implementation heavily borrowed from testutil.TreeDump func reportFromRootfs(rootDir string) (*Report, error) { @@ -768,69 +568,3 @@ func reportFromRootfs(rootDir string) (*Report, error) { return report, nil } - -// reportFromManifest builds a Report from a Manifest -func reportFromManifest(m *manifest.Manifest) (*Report, error) { - report := &Report{ - Entries: make(map[string]ReportEntry), - } - - err := m.IteratePaths("", func(path *manifest.Path) error { - mode, err := strconv.ParseUint(path.Mode, 8, 32) - if err != nil { - return fmt.Errorf("cannot parse mode: %v", err) - } - - entry := ReportEntry{ - Path: path.Path, - Mode: fs.FileMode(mode), - Size: int(path.Size), - FinalSHA256: path.FinalSHA256, - SHA256: path.SHA256, - Inode: path.Inode, - Link: path.Link, - } - report.Entries[path.Path] = entry - - return nil - }) - - return report, err -} - -// validateReportEntries validates report entries of toValidate against -// entries in the reference. The report under validation can contain more -// entries. -func validateReportEntries(reference, toValidate *Report) error { - for path, referenceEntry := range reference.Entries { - var errorMessage string - entryToValidate, ok := toValidate.Entries[path] - if !ok { - return fmt.Errorf("%q is missing", path) - } - - if entryToValidate.Mode != referenceEntry.Mode { - errorMessage = fmt.Sprintf("invalid mode: expected %q, found %q", referenceEntry.Mode, entryToValidate.Mode) - } - if entryToValidate.Size != referenceEntry.Size { - errorMessage = fmt.Sprintf("invalid size: expected %d, found %d", referenceEntry.Size, entryToValidate.Size) - } - if len(referenceEntry.FinalSHA256) > 0 { - if entryToValidate.SHA256 != referenceEntry.FinalSHA256 { - errorMessage = fmt.Sprintf("invalid hash: expected %s, found %s", referenceEntry.FinalSHA256, entryToValidate.SHA256) - } - } else if len(referenceEntry.SHA256) > 0 && entryToValidate.SHA256 != referenceEntry.SHA256 { - errorMessage = fmt.Sprintf("invalid hash: expected %s, found %s", referenceEntry.SHA256, entryToValidate.SHA256) - } - - if entryToValidate.Link != referenceEntry.Link { - errorMessage = fmt.Sprintf("invalid link: expected %q, found %q", referenceEntry.Link, entryToValidate.Link) - } - - if len(errorMessage) > 0 { - return fmt.Errorf("invalid entry %q: %s", path, errorMessage) - } - } - - return nil -} diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go new file mode 100644 index 00000000..02475f8e --- /dev/null +++ b/internal/manifestutil/verify.go @@ -0,0 +1,194 @@ +package manifestutil + +import ( + "encoding/hex" + "fmt" + "os" + "path/filepath" + "slices" + "strings" + "syscall" + + "github.com/canonical/chisel/public/manifest" +) + +// validateRootfs verify the content of the target directory is in line with +// the manifest. +// This function works under the assumption the manifest was previously +// validated. +// Files not managed by chisel are not checked. +func validateRootfs(m *manifest.Manifest, rootDir string) error { + pathGroups, err := groupManifestPaths(m) + if err != nil { + return err + } + + for _, group := range pathGroups { + err := verifyGroup(rootDir, group) + if err != nil { + return err + } + } + + return nil +} + +type pathGroup struct { + headEntry *manifest.Path + paths []string +} + +// groupManifestPaths groups paths by inode. +func groupManifestPaths(m *manifest.Manifest) (map[string]*pathGroup, error) { + pathGroups := make(map[string]*pathGroup) + inodeGroups := make(map[uint64]string) + + err := m.IteratePaths("", func(path *manifest.Path) error { + inode := path.Inode + if inode == 0 { + // New single path + pathGroups[path.Path] = &pathGroup{ + headEntry: path, + paths: []string{path.Path}, + } + return nil + } + + groupKey, ok := inodeGroups[inode] + if !ok { + // New group of hardlink + pathGroups[path.Path] = &pathGroup{ + headEntry: path, + paths: []string{path.Path}, + } + inodeGroups[inode] = path.Path + return nil + } + // Add path to the existing group + group := pathGroups[groupKey] + group.paths = append(group.paths, path.Path) + + return nil + }) + if err != nil { + return nil, err + } + + // sort paths + for _, group := range pathGroups { + slices.Sort(group.paths) + } + + return pathGroups, nil +} + +// verifyGroup verifies a group of paths +func verifyGroup(rootDir string, group *pathGroup) error { + path := group.headEntry + + info, err := verifyPath(rootDir, path) + if err != nil { + return err + } + + // Verify hardlinks + if len(group.paths) == 0 { + // Not a hardlink, nothing more to do + return nil + } + + headInode, err := getPhysicalInode(info) + if err != nil { + return err + } + + return verifyHardlinks(headInode, rootDir, path.Path, group.paths) +} + +func verifyPath(rootDir string, path *manifest.Path) (os.FileInfo, error) { + p := filepath.Join(rootDir, path.Path) + info, err := os.Lstat(p) + if err != nil { + return nil, err + } + mode := info.Mode() + if fmt.Sprintf("0%o", unixPerm(mode)) != path.Mode { + return nil, fmt.Errorf("tampered content: %q mode mismatch: %s recorded, %s observed", path.Path, path.Mode, mode.String()) + } + + // Verify directories + if strings.HasSuffix(path.Path, "/") { + if !info.IsDir() { + return nil, fmt.Errorf("tampered content: %q expected to be a directory", path.Path) + } + return info, nil + } + + // Verify symlinks + if len(path.Link) > 0 { + link, err := os.Readlink(p) + if err != nil { + return nil, err + } + + if link != path.Link { + return nil, fmt.Errorf("tampered content: %q link destination mismatch: %s recorded, %s observed", path.Path, path.Link, link) + } + return info, nil + } + + if !mode.IsRegular() { + return nil, fmt.Errorf("tampered content: unrecognized type of file.") + } + + // Verify size + if info.Size() != int64(path.Size) { + return nil, fmt.Errorf("tampered file: %q, size mimsmatch: %d recorded, %d observed", path.Path, info.Size(), int64(path.Size)) + } + + // Verify hash + // Most expensive operation, so do it at the end. + h, err := hash(p) + if err != nil { + return nil, err + } + hString := hex.EncodeToString(h) + if len(path.FinalSHA256) > 0 { + if hString != path.FinalSHA256 { + return nil, fmt.Errorf("tampered file %q, hash mismatch: %s recorded, %s observed", path.Path, path.FinalSHA256, hString) + } + } else if len(path.SHA256) > 0 && hString != path.SHA256 { + // This is effectively skipping the manifest.wall since no hash is declared on it + return nil, fmt.Errorf("tampered file %q, sha256 mismatch: %s recorded, %s observed", path.Path, path.SHA256, hString) + } + return info, nil +} + +func verifyHardlinks(headInode uint64, rootDir string, headPath string, paths []string) error { + for _, siblingPath := range paths[1:] { + siblingFullPath := filepath.Join(rootDir, siblingPath) + sibFi, err := os.Lstat(siblingFullPath) + if err != nil { + return err + } + + // Verify Inode Equality + sibInode, err := getPhysicalInode(sibFi) + if err != nil { + return err + } + + if sibInode != headInode { + return fmt.Errorf("tampered content: broken hardlink: %s and %s should share inode but do not", headPath, siblingPath) + } + } + return nil +} + +func getPhysicalInode(info os.FileInfo) (uint64, error) { + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return 0, fmt.Errorf("cannot get syscall stat info for %q", info.Name()) + } + return stat.Ino, nil +} From 70b2c5ec9ebf16355222f3914e53532b1a418b48 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 2 Dec 2025 11:26:47 +0100 Subject: [PATCH 11/54] refactor: rework verify* funcs Signed-off-by: Paul Mars --- internal/manifestutil/verify.go | 70 ++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index 02475f8e..896b3806 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -86,98 +86,104 @@ func groupManifestPaths(m *manifest.Manifest) (map[string]*pathGroup, error) { func verifyGroup(rootDir string, group *pathGroup) error { path := group.headEntry - info, err := verifyPath(rootDir, path) + fpath := filepath.Join(rootDir, path.Path) + info, err := os.Lstat(fpath) if err != nil { return err } - // Verify hardlinks - if len(group.paths) == 0 { - // Not a hardlink, nothing more to do - return nil - } - - headInode, err := getPhysicalInode(info) + err = verifyPath(info, fpath, path) if err != nil { return err } - return verifyHardlinks(headInode, rootDir, path.Path, group.paths) + return verifyHardlinks(info, path.Path, rootDir, group.paths) } -func verifyPath(rootDir string, path *manifest.Path) (os.FileInfo, error) { - p := filepath.Join(rootDir, path.Path) - info, err := os.Lstat(p) - if err != nil { - return nil, err - } +// verifyPath verifies a single path against its manifest entry +func verifyPath(info os.FileInfo, fpath string, path *manifest.Path) error { mode := info.Mode() + + // Verify mode if fmt.Sprintf("0%o", unixPerm(mode)) != path.Mode { - return nil, fmt.Errorf("tampered content: %q mode mismatch: %s recorded, %s observed", path.Path, path.Mode, mode.String()) + return fmt.Errorf("tampered content: %q mode mismatch: %s recorded, %s observed", path.Path, path.Mode, mode.String()) } // Verify directories if strings.HasSuffix(path.Path, "/") { if !info.IsDir() { - return nil, fmt.Errorf("tampered content: %q expected to be a directory", path.Path) + return fmt.Errorf("tampered content: %q expected to be a directory", path.Path) } - return info, nil + return nil } // Verify symlinks if len(path.Link) > 0 { - link, err := os.Readlink(p) + link, err := os.Readlink(fpath) if err != nil { - return nil, err + return err } if link != path.Link { - return nil, fmt.Errorf("tampered content: %q link destination mismatch: %s recorded, %s observed", path.Path, path.Link, link) + return fmt.Errorf("tampered content: %q link destination mismatch: %s recorded, %s observed", path.Path, path.Link, link) } - return info, nil + return nil } if !mode.IsRegular() { - return nil, fmt.Errorf("tampered content: unrecognized type of file.") + return fmt.Errorf("tampered content: unrecognized type of file.") } // Verify size if info.Size() != int64(path.Size) { - return nil, fmt.Errorf("tampered file: %q, size mimsmatch: %d recorded, %d observed", path.Path, info.Size(), int64(path.Size)) + return fmt.Errorf("tampered file: %q, size mimsmatch: %d recorded, %d observed", path.Path, info.Size(), int64(path.Size)) } // Verify hash // Most expensive operation, so do it at the end. - h, err := hash(p) + h, err := hash(fpath) if err != nil { - return nil, err + return err } hString := hex.EncodeToString(h) if len(path.FinalSHA256) > 0 { if hString != path.FinalSHA256 { - return nil, fmt.Errorf("tampered file %q, hash mismatch: %s recorded, %s observed", path.Path, path.FinalSHA256, hString) + return fmt.Errorf("tampered file %q, hash mismatch: %s recorded, %s observed", path.Path, path.FinalSHA256, hString) } } else if len(path.SHA256) > 0 && hString != path.SHA256 { // This is effectively skipping the manifest.wall since no hash is declared on it - return nil, fmt.Errorf("tampered file %q, sha256 mismatch: %s recorded, %s observed", path.Path, path.SHA256, hString) + return fmt.Errorf("tampered file %q, sha256 mismatch: %s recorded, %s observed", path.Path, path.SHA256, hString) } - return info, nil + return nil } -func verifyHardlinks(headInode uint64, rootDir string, headPath string, paths []string) error { - for _, siblingPath := range paths[1:] { +func verifyHardlinks(headInfo os.FileInfo, headPath string, rootDir string, paths []string) error { + if len(paths) == 0 { + // No hardlinks, nothing to do + return nil + } + + headInode, err := getPhysicalInode(headInfo) + if err != nil { + return err + } + + for _, siblingPath := range paths { + if siblingPath == headPath { + continue + } siblingFullPath := filepath.Join(rootDir, siblingPath) sibFi, err := os.Lstat(siblingFullPath) if err != nil { return err } - // Verify Inode Equality sibInode, err := getPhysicalInode(sibFi) if err != nil { return err } + // Verify Inode Equality if sibInode != headInode { return fmt.Errorf("tampered content: broken hardlink: %s and %s should share inode but do not", headPath, siblingPath) } From c8b7a39b738d4cad0da1cfc2869c3190e86420b4 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 2 Dec 2025 13:57:10 +0100 Subject: [PATCH 12/54] fix: typos and inconsistencies Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 2 +- internal/manifestutil/manifestutil.go | 21 ++++++++++++--------- internal/manifestutil/verify.go | 2 +- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 3fd8c181..5db5c87e 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -88,7 +88,7 @@ func (cmd *cmdCut) Execute(args []string) error { logf("Warning: %v", err) } else { // Merge the slice keys used to build the existing rootfs with the ones - // explicitly requested slice. + // explicitly requested slices. // Previous slice keys contains both explicitly requested slices and slices // resolved following essentials. for _, s := range manifestutil.SliceKeys(manifest) { diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index dacb6320..55b9c76c 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -376,13 +376,14 @@ func RootFSManifest(release *setup.Release, targetDir string) (*manifest.Manifes // Select the first manifest of the list as the reference one for now. // Another heuristic could be used (ex. select the one from base-files_chisel). - refManifestPath := path.Join(targetDir, manifestPaths[0]) + refManifestRelPath := manifestPaths[0] + refManifestPath := path.Join(targetDir, refManifestRelPath) refManifest, err := load(refManifestPath) if err != nil { - return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", refManifestPath, err) + return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", refManifestRelPath, err) } - err = checkConsistency(refManifestPath, targetDir, manifestPaths[1:]) + err = checkConsistency(refManifestRelPath, targetDir, manifestPaths[1:]) if err != nil { return nil, err } @@ -423,7 +424,8 @@ func load(manifestPath string) (*manifest.Manifest, error) { // checkConsistency checks consistency between a list of manifests and a // reference one. -func checkConsistency(reference string, targetDir string, manifests []string) error { +func checkConsistency(referenceRelPath string, targetDir string, manifests []string) error { + reference := path.Join(targetDir, referenceRelPath) hashReference, err := hash(reference) if err != nil { return err @@ -436,23 +438,24 @@ func checkConsistency(reference string, targetDir string, manifests []string) er modeRef := infoRef.Mode() - for _, m := range manifests { - infoManifest, err := os.Stat(path.Join(targetDir, m)) + for _, manifestRelPath := range manifests { + manifestPath := path.Join(targetDir, manifestRelPath) + infoManifest, err := os.Stat(manifestPath) if err != nil { return err } modeManifest := infoManifest.Mode() if modeManifest != modeRef { - return fmt.Errorf("invalid manifest: permissions on %s (%s) are different from the reference manifest %s (%s)", m, modeManifest, reference, modeRef) + return fmt.Errorf("invalid manifest: permissions on %s (%s) are different from the reference manifest %s (%s)", manifestRelPath, modeManifest, referenceRelPath, modeRef) } - hashM, err := hash(m) + hashM, err := hash(manifestPath) if err != nil { return err } if !slices.Equal(hashM, hashReference) { - return fmt.Errorf("invalid manifest: %s is inconsistent with %s", m, reference) + return fmt.Errorf("invalid manifest: %s is inconsistent with %s", manifestRelPath, referenceRelPath) } } diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index 896b3806..0f7b5085 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -136,7 +136,7 @@ func verifyPath(info os.FileInfo, fpath string, path *manifest.Path) error { // Verify size if info.Size() != int64(path.Size) { - return fmt.Errorf("tampered file: %q, size mimsmatch: %d recorded, %d observed", path.Path, info.Size(), int64(path.Size)) + return fmt.Errorf("tampered file: %q, size mismatch: %d recorded, %d observed", path.Path, info.Size(), int64(path.Size)) } // Verify hash From 1181871a4b49e178f60e79e4f12cba846b196b04 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 2 Dec 2025 15:46:37 +0100 Subject: [PATCH 13/54] refactor: improve readability Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 2 +- internal/manifestutil/manifestutil.go | 102 ++------------ internal/manifestutil/verify.go | 193 ++++++++++++++++++++------ 3 files changed, 159 insertions(+), 138 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 5db5c87e..b5bfd87a 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -89,7 +89,7 @@ func (cmd *cmdCut) Execute(args []string) error { } else { // Merge the slice keys used to build the existing rootfs with the ones // explicitly requested slices. - // Previous slice keys contains both explicitly requested slices and slices + // Previous slice keys contain both explicitly requested slices and slices // resolved following essentials. for _, s := range manifestutil.SliceKeys(manifest) { if !slices.Contains(sliceKeys, s) { diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 55b9c76c..a20ce42e 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -2,7 +2,6 @@ package manifestutil import ( "crypto/sha256" - "encoding/hex" "fmt" "io" "io/fs" @@ -12,7 +11,6 @@ import ( "slices" "sort" "strings" - "syscall" "github.com/canonical/chisel/internal/apacheutil" "github.com/canonical/chisel/internal/archive" @@ -43,19 +41,21 @@ func FindPaths(slices []*setup.Slice) map[string][]*setup.Slice { // FindPathsInRelease finds all the paths marked with "generate:manifest" // for the given release. func FindPathsInRelease(r *setup.Release) []string { - manifests := []string{} + allSlices := []*setup.Slice{} for _, pkg := range r.Packages { for _, slice := range pkg.Slices { - for path, info := range slice.Contents { - if info.Generate == setup.GenerateManifest { - dir := strings.TrimSuffix(path, "**") - path = filepath.Join(dir, DefaultFilename) - manifests = append(manifests, path) - } - } + allSlices = append(allSlices, slice) } } - return manifests + + manifestMap := FindPaths(allSlices) + + paths := make([]string, 0, len(manifestMap)) + for path := range manifestMap { + paths = append(paths, path) + } + + return paths } type WriteOptions struct { @@ -491,83 +491,3 @@ func SliceKeys(m *manifest.Manifest) []setup.SliceKey { return sliceKeys } - -// reportFromRootfs builds a Report from root directory -// Implementation heavily borrowed from testutil.TreeDump -func reportFromRootfs(rootDir string) (*Report, error) { - report, err := NewReport(rootDir) - if err != nil { - return nil, fmt.Errorf("internal error: cannot create report: %w", err) - } - - var inodes []uint64 - pathsByInodes := make(map[uint64][]string) - - dirfs := os.DirFS(report.Root) - err = fs.WalkDir(dirfs, ".", func(path string, d fs.DirEntry, err error) error { - if err != nil { - return fmt.Errorf("walk error: %w", err) - } - if path == "." { - return nil - } - fpath := filepath.Join(report.Root, path) - finfo, err := d.Info() - if err != nil { - return fmt.Errorf("cannot get stat info for %q: %w", fpath, err) - } - - entry := ReportEntry{ - Mode: finfo.Mode(), - } - - var size int - - ftype := finfo.Mode() & fs.ModeType - switch ftype { - case fs.ModeDir: - path = "/" + path + "/" - case fs.ModeSymlink: - lpath, err := os.Readlink(fpath) - if err != nil { - return err - } - path = "/" + path - entry.Link = lpath - case 0: // Regular - data, err := os.ReadFile(fpath) - if err != nil { - return fmt.Errorf("cannot read file: %w", err) - } - if len(data) >= 0 { - sum := sha256.Sum256(data) - entry.SHA256 = hex.EncodeToString(sum[:]) - } - path = "/" + path - size = int(finfo.Size()) - default: - return fmt.Errorf("unknown file type %d: %s", ftype, fpath) - } - entry.Path = path - entry.Size = size - - if ftype != fs.ModeDir { - stat, ok := finfo.Sys().(*syscall.Stat_t) - if !ok { - return fmt.Errorf("cannot get syscall stat info for %q", fpath) - } - inode := stat.Ino - if len(pathsByInodes[inode]) == 1 { - inodes = append(inodes, inode) - } - entry.Inode = inode - pathsByInodes[inode] = append(pathsByInodes[inode], path) - } - - report.Entries[path] = entry - - return nil - }) - - return report, nil -} diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index 0f7b5085..06bb80a7 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -39,34 +39,39 @@ type pathGroup struct { } // groupManifestPaths groups paths by inode. -func groupManifestPaths(m *manifest.Manifest) (map[string]*pathGroup, error) { - pathGroups := make(map[string]*pathGroup) - inodeGroups := make(map[uint64]string) +// Returns a slice of path groups where each group represents either: +// - A standalone file (inode 0) +// - A set of hardlinked files (same non-zero inode) +func groupManifestPaths(m *manifest.Manifest) ([]*pathGroup, error) { + pathGroups := []*pathGroup{} + inodeToGroup := make(map[uint64]*pathGroup) err := m.IteratePaths("", func(path *manifest.Path) error { inode := path.Inode if inode == 0 { - // New single path - pathGroups[path.Path] = &pathGroup{ + // Standalone path + pathGroups = append(pathGroups, &pathGroup{ headEntry: path, paths: []string{path.Path}, - } + }) return nil } - groupKey, ok := inodeGroups[inode] + // Hardlinked path + group, ok := inodeToGroup[inode] if !ok { - // New group of hardlink - pathGroups[path.Path] = &pathGroup{ + // New group of hardlinks + group = &pathGroup{ headEntry: path, paths: []string{path.Path}, } - inodeGroups[inode] = path.Path + inodeToGroup[inode] = group + pathGroups = append(pathGroups, group) return nil + } else { + // Add path to the existing group + group.paths = append(group.paths, path.Path) } - // Add path to the existing group - group := pathGroups[groupKey] - group.paths = append(group.paths, path.Path) return nil }) @@ -76,7 +81,9 @@ func groupManifestPaths(m *manifest.Manifest) (map[string]*pathGroup, error) { // sort paths for _, group := range pathGroups { - slices.Sort(group.paths) + if len(group.paths) > 1 { + slices.Sort(group.paths) + } } return pathGroups, nil @@ -92,8 +99,7 @@ func verifyGroup(rootDir string, group *pathGroup) error { return err } - err = verifyPath(info, fpath, path) - if err != nil { + if err := verifyPath(info, fpath, path); err != nil { return err } @@ -104,59 +110,153 @@ func verifyGroup(rootDir string, group *pathGroup) error { func verifyPath(info os.FileInfo, fpath string, path *manifest.Path) error { mode := info.Mode() - // Verify mode - if fmt.Sprintf("0%o", unixPerm(mode)) != path.Mode { - return fmt.Errorf("tampered content: %q mode mismatch: %s recorded, %s observed", path.Path, path.Mode, mode.String()) + if err := verifyFileType(info, path); err != nil { + return err + } + + if err := verifyMode(mode, path); err != nil { + return err } - // Verify directories if strings.HasSuffix(path.Path, "/") { - if !info.IsDir() { - return fmt.Errorf("tampered content: %q expected to be a directory", path.Path) - } + // Directories have no additional checks return nil } - // Verify symlinks if len(path.Link) > 0 { - link, err := os.Readlink(fpath) - if err != nil { - return err - } - - if link != path.Link { - return fmt.Errorf("tampered content: %q link destination mismatch: %s recorded, %s observed", path.Path, path.Link, link) - } - return nil + return verifySymlink(fpath, path) } if !mode.IsRegular() { - return fmt.Errorf("tampered content: unrecognized type of file.") + return fmt.Errorf("tampered content: %q has unrecognized type %s.", path.Path, mode.String()) } - // Verify size - if info.Size() != int64(path.Size) { - return fmt.Errorf("tampered file: %q, size mismatch: %d recorded, %d observed", path.Path, info.Size(), int64(path.Size)) + if err := verifySize(info, path); err != nil { + return err } // Verify hash // Most expensive operation, so do it at the end. + return verifyHash(fpath, path) +} + +// verifyFileType checks that the file type matches expectations. +func verifyFileType(info os.FileInfo, path *manifest.Path) error { + mode := info.Mode() + isDir := strings.HasSuffix(path.Path, "/") + isSymlink := path.Link != "" + + if isDir && !info.IsDir() { + return fmt.Errorf("tampered content: %q expected to be a directory but found %s", + path.Path, mode.Type().String()) + } + + if !isDir && info.IsDir() { + return fmt.Errorf("tampered content: %q is a directory but manifest expects a file", + path.Path) + } + + if isSymlink && mode.Type() != os.ModeSymlink { + return fmt.Errorf("tampered content: %q expected to be a symlink but found %s", + path.Path, mode.Type().String()) + } + + if !isSymlink && mode.Type() == os.ModeSymlink { + return fmt.Errorf("tampered content: %q is a symlink but manifest expects regular file", + path.Path) + } + + return nil +} + +// verifyMode checks file permissions match the manifest. +func verifyMode(mode os.FileMode, path *manifest.Path) error { + if path.Mode == "" { + return fmt.Errorf("internal error: missing mode for path %q", path.Path) + } + + expectedMode := path.Mode + actualMode := fmt.Sprintf("0%o", unixPerm(mode)) + + if actualMode != expectedMode { + return fmt.Errorf("tampered content: %q mode mismatch: expected %s, observed %s", + path.Path, expectedMode, actualMode) + } + + return nil +} + +// verifySymlink checks symlink target matches the manifest. +func verifySymlink(fpath string, path *manifest.Path) error { + if path.Link == "" { + return fmt.Errorf("internal error: path %q marked as symlink but no target specified", path.Path) + } + + link, err := os.Readlink(fpath) + if err != nil { + return fmt.Errorf("cannot read symlink %q: %w", path.Path, err) + } + + if link != path.Link { + return fmt.Errorf("tampered content: %q symlink mismatch: expected %q → %q, observed %q → %q", + path.Path, path.Path, path.Link, path.Path, link) + } + + return nil +} + +// verifySize checks file size matches the manifest. +func verifySize(info os.FileInfo, path *manifest.Path) error { + expected := int64(path.Size) + actual := info.Size() + + if actual != expected { + return fmt.Errorf("tampered file: %q size mismatch: expected %d bytes, observed %d bytes", + path.Path, expected, actual) + } + + return nil +} + +// verifyHash verifies file content hash. +// Uses FinalSHA256 if present (post-mutation hash), otherwise SHA256 (original file hash). +// Files without any hash declaration are skipped (e.g., manifest.wall). +func verifyHash(fpath string, path *manifest.Path) error { + expectedHash := path.FinalSHA256 + hashType := "final" + + if expectedHash == "" { + expectedHash = path.SHA256 + hashType = "original" + } + + // Skip hash verification if no hash is declared + if expectedHash == "" { + // This is skipping manifest.wall that is generated + // during the cut operation and have no predetermined hash + return nil + } + + if len(expectedHash) != 64 { + return fmt.Errorf("internal error: invalid SHA256 hash length for %q: %d", + path.Path, len(expectedHash)) + } + h, err := hash(fpath) if err != nil { - return err + return fmt.Errorf("cannot compute hash for %q: %w", path.Path, err) } - hString := hex.EncodeToString(h) - if len(path.FinalSHA256) > 0 { - if hString != path.FinalSHA256 { - return fmt.Errorf("tampered file %q, hash mismatch: %s recorded, %s observed", path.Path, path.FinalSHA256, hString) - } - } else if len(path.SHA256) > 0 && hString != path.SHA256 { - // This is effectively skipping the manifest.wall since no hash is declared on it - return fmt.Errorf("tampered file %q, sha256 mismatch: %s recorded, %s observed", path.Path, path.SHA256, hString) + + actualHash := hex.EncodeToString(h) + if actualHash != expectedHash { + return fmt.Errorf("tampered file: %q %s hash mismatch: expected %s, observed %s", + path.Path, hashType, expectedHash, actualHash) } + return nil } +// verifyHardlinks verifies that all paths in the list share the same inode. func verifyHardlinks(headInfo os.FileInfo, headPath string, rootDir string, paths []string) error { if len(paths) == 0 { // No hardlinks, nothing to do @@ -191,6 +291,7 @@ func verifyHardlinks(headInfo os.FileInfo, headPath string, rootDir string, path return nil } +// getPhysicalInode retrieves the inode number from os.FileInfo func getPhysicalInode(info os.FileInfo) (uint64, error) { stat, ok := info.Sys().(*syscall.Stat_t) if !ok { From 31f1d3d3c48b7fef1e3afe9c945755ee542cbf21 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 3 Dec 2025 14:00:21 +0100 Subject: [PATCH 14/54] refactor: improve consistency Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 15 ++++-- internal/manifestutil/manifestutil.go | 34 ++++++------ internal/manifestutil/verify.go | 78 +++++++++++---------------- 3 files changed, 57 insertions(+), 70 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index b5bfd87a..1a3e1cdb 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -76,17 +76,22 @@ func (cmd *cmdCut) Execute(args []string) error { } } - rootfsPath, err := preExistingRootfs(cmd.RootDir) + targetDir, err := nonEmptyDir(cmd.RootDir) if err != nil { return err } - if len(rootfsPath) > 0 { - manifest, err := manifestutil.RootFSManifest(release, rootfsPath) + if len(targetDir) > 0 { + manifest, err := manifestutil.FromDir(release, targetDir) if err != nil { // TODO: When enabling the feature, error out. logf("Warning: %v", err) } else { + err = manifestutil.VerifyDir(manifest, targetDir) + if err != nil { + // TODO: When enabling the feature, error out. + logf("Warning: %v", err) + } // Merge the slice keys used to build the existing rootfs with the ones // explicitly requested slices. // Previous slice keys contain both explicitly requested slices and slices @@ -155,8 +160,8 @@ func (cmd *cmdCut) Execute(args []string) error { return err } -// preExistingRootfs determines if the target directory was produced by chisel. -func preExistingRootfs(rootdir string) (string, error) { +// nonEmptyDir checks whether the given directory exists and is non-empty. +func nonEmptyDir(rootdir string) (string, error) { // Get targetDir path // Note: This is already done in `slicer.Run`. Extract it and avoid doing it twice? targetDir := filepath.Clean(rootdir) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index a20ce42e..55b55b90 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -365,34 +365,30 @@ func Validate(mfest *manifest.Manifest) (err error) { return nil } -// RootFSManifest extracts the manifest from a targetDir -func RootFSManifest(release *setup.Release, targetDir string) (*manifest.Manifest, error) { +// FromDir extracts, validates and returns the manifest from a targetDir +func FromDir(release *setup.Release, targetDir string) (*manifest.Manifest, error) { manifestPaths := FindPathsInRelease(release) if len(manifestPaths) == 0 { // No manifest in the release means it cannot produce a rootfs that can // be recut. Treat this case as cutting a new rootfs. - return nil, nil + return nil, fmt.Errorf("no manifest generated for this release") } // Select the first manifest of the list as the reference one for now. // Another heuristic could be used (ex. select the one from base-files_chisel). - refManifestRelPath := manifestPaths[0] - refManifestPath := path.Join(targetDir, refManifestRelPath) - refManifest, err := load(refManifestPath) + referenceRelPath := manifestPaths[0] + referencePath := path.Join(targetDir, referenceRelPath) + reference, err := load(referencePath) if err != nil { - return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", refManifestRelPath, err) + return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", referenceRelPath, err) } - err = checkConsistency(refManifestRelPath, targetDir, manifestPaths[1:]) + err = checkConsistency(referenceRelPath, targetDir, manifestPaths[1:]) if err != nil { return nil, err } - err = validateRootfs(refManifest, targetDir) - if err != nil { - return nil, err - } - return refManifest, nil + return reference, nil } // load reads, validates and returns a manifest. @@ -428,12 +424,12 @@ func checkConsistency(referenceRelPath string, targetDir string, manifests []str reference := path.Join(targetDir, referenceRelPath) hashReference, err := hash(reference) if err != nil { - return err + return fmt.Errorf("internal error: cannot compute hash for %q: %w", referenceRelPath, err) } infoRef, err := os.Stat(reference) if err != nil { - return err + return fmt.Errorf("internal error: cannot get file info for %q: %w", referenceRelPath, err) } modeRef := infoRef.Mode() @@ -442,7 +438,7 @@ func checkConsistency(referenceRelPath string, targetDir string, manifests []str manifestPath := path.Join(targetDir, manifestRelPath) infoManifest, err := os.Stat(manifestPath) if err != nil { - return err + return fmt.Errorf("internal error: cannot get file info for %q: %w", manifestRelPath, err) } modeManifest := infoManifest.Mode() @@ -452,7 +448,7 @@ func checkConsistency(referenceRelPath string, targetDir string, manifests []str hashM, err := hash(manifestPath) if err != nil { - return err + return fmt.Errorf("internal error: cannot compute hash for %q: %w", manifestRelPath, err) } if !slices.Equal(hashM, hashReference) { return fmt.Errorf("invalid manifest: %s is inconsistent with %s", manifestRelPath, referenceRelPath) @@ -478,9 +474,9 @@ func hash(path string) ([]byte, error) { } // SliceKeys returns setup.SliceKeys from a manifest. -func SliceKeys(m *manifest.Manifest) []setup.SliceKey { +func SliceKeys(mfest *manifest.Manifest) []setup.SliceKey { sliceKeys := []setup.SliceKey{} - m.IterateSlices("", func(slice *manifest.Slice) error { + mfest.IterateSlices("", func(slice *manifest.Slice) error { sk, err := apacheutil.ParseSliceKey(slice.Name) if err != nil { return err diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index 06bb80a7..748a8813 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -12,19 +12,18 @@ import ( "github.com/canonical/chisel/public/manifest" ) -// validateRootfs verify the content of the target directory is in line with +// VerifyDir verifies the content of the target directory matches with // the manifest. -// This function works under the assumption the manifest was previously -// validated. -// Files not managed by chisel are not checked. -func validateRootfs(m *manifest.Manifest, rootDir string) error { - pathGroups, err := groupManifestPaths(m) +// This function works under the assumption the manifest is valid. +// Files not managed by chisel are ignored. +func VerifyDir(mfest *manifest.Manifest, rootDir string) error { + pathGroups, err := groupPaths(mfest) if err != nil { return err } for _, group := range pathGroups { - err := verifyGroup(rootDir, group) + err := verifyGroup(group, rootDir) if err != nil { return err } @@ -34,25 +33,25 @@ func validateRootfs(m *manifest.Manifest, rootDir string) error { } type pathGroup struct { - headEntry *manifest.Path - paths []string + head *manifest.Path + paths []string } -// groupManifestPaths groups paths by inode. +// groupPaths groups paths by inode. // Returns a slice of path groups where each group represents either: // - A standalone file (inode 0) // - A set of hardlinked files (same non-zero inode) -func groupManifestPaths(m *manifest.Manifest) ([]*pathGroup, error) { +func groupPaths(mfest *manifest.Manifest) ([]*pathGroup, error) { pathGroups := []*pathGroup{} inodeToGroup := make(map[uint64]*pathGroup) - err := m.IteratePaths("", func(path *manifest.Path) error { + err := mfest.IteratePaths("", func(path *manifest.Path) error { inode := path.Inode if inode == 0 { // Standalone path pathGroups = append(pathGroups, &pathGroup{ - headEntry: path, - paths: []string{path.Path}, + head: path, + paths: []string{path.Path}, }) return nil } @@ -62,8 +61,8 @@ func groupManifestPaths(m *manifest.Manifest) ([]*pathGroup, error) { if !ok { // New group of hardlinks group = &pathGroup{ - headEntry: path, - paths: []string{path.Path}, + head: path, + paths: []string{path.Path}, } inodeToGroup[inode] = group pathGroups = append(pathGroups, group) @@ -90,8 +89,8 @@ func groupManifestPaths(m *manifest.Manifest) ([]*pathGroup, error) { } // verifyGroup verifies a group of paths -func verifyGroup(rootDir string, group *pathGroup) error { - path := group.headEntry +func verifyGroup(group *pathGroup, rootDir string) error { + path := group.head fpath := filepath.Join(rootDir, path.Path) info, err := os.Lstat(fpath) @@ -99,7 +98,7 @@ func verifyGroup(rootDir string, group *pathGroup) error { return err } - if err := verifyPath(info, fpath, path); err != nil { + if err := verifyPath(path, info, fpath); err != nil { return err } @@ -107,14 +106,14 @@ func verifyGroup(rootDir string, group *pathGroup) error { } // verifyPath verifies a single path against its manifest entry -func verifyPath(info os.FileInfo, fpath string, path *manifest.Path) error { +func verifyPath(path *manifest.Path, info os.FileInfo, fpath string) error { mode := info.Mode() - if err := verifyFileType(info, path); err != nil { + if err := verifyFileType(path, info); err != nil { return err } - if err := verifyMode(mode, path); err != nil { + if err := verifyMode(path, mode); err != nil { return err } @@ -124,24 +123,24 @@ func verifyPath(info os.FileInfo, fpath string, path *manifest.Path) error { } if len(path.Link) > 0 { - return verifySymlink(fpath, path) + return verifySymlink(path, fpath) } if !mode.IsRegular() { return fmt.Errorf("tampered content: %q has unrecognized type %s.", path.Path, mode.String()) } - if err := verifySize(info, path); err != nil { + if err := verifySize(path, info); err != nil { return err } // Verify hash // Most expensive operation, so do it at the end. - return verifyHash(fpath, path) + return verifyHash(path, fpath) } // verifyFileType checks that the file type matches expectations. -func verifyFileType(info os.FileInfo, path *manifest.Path) error { +func verifyFileType(path *manifest.Path, info os.FileInfo) error { mode := info.Mode() isDir := strings.HasSuffix(path.Path, "/") isSymlink := path.Link != "" @@ -170,11 +169,7 @@ func verifyFileType(info os.FileInfo, path *manifest.Path) error { } // verifyMode checks file permissions match the manifest. -func verifyMode(mode os.FileMode, path *manifest.Path) error { - if path.Mode == "" { - return fmt.Errorf("internal error: missing mode for path %q", path.Path) - } - +func verifyMode(path *manifest.Path, mode os.FileMode) error { expectedMode := path.Mode actualMode := fmt.Sprintf("0%o", unixPerm(mode)) @@ -187,11 +182,7 @@ func verifyMode(mode os.FileMode, path *manifest.Path) error { } // verifySymlink checks symlink target matches the manifest. -func verifySymlink(fpath string, path *manifest.Path) error { - if path.Link == "" { - return fmt.Errorf("internal error: path %q marked as symlink but no target specified", path.Path) - } - +func verifySymlink(path *manifest.Path, fpath string) error { link, err := os.Readlink(fpath) if err != nil { return fmt.Errorf("cannot read symlink %q: %w", path.Path, err) @@ -206,7 +197,7 @@ func verifySymlink(fpath string, path *manifest.Path) error { } // verifySize checks file size matches the manifest. -func verifySize(info os.FileInfo, path *manifest.Path) error { +func verifySize(path *manifest.Path, info os.FileInfo) error { expected := int64(path.Size) actual := info.Size() @@ -219,9 +210,9 @@ func verifySize(info os.FileInfo, path *manifest.Path) error { } // verifyHash verifies file content hash. -// Uses FinalSHA256 if present (post-mutation hash), otherwise SHA256 (original file hash). +// Uses FinalSHA256 if present, otherwise SHA256. // Files without any hash declaration are skipped (e.g., manifest.wall). -func verifyHash(fpath string, path *manifest.Path) error { +func verifyHash(path *manifest.Path, fpath string) error { expectedHash := path.FinalSHA256 hashType := "final" @@ -237,14 +228,9 @@ func verifyHash(fpath string, path *manifest.Path) error { return nil } - if len(expectedHash) != 64 { - return fmt.Errorf("internal error: invalid SHA256 hash length for %q: %d", - path.Path, len(expectedHash)) - } - h, err := hash(fpath) if err != nil { - return fmt.Errorf("cannot compute hash for %q: %w", path.Path, err) + return fmt.Errorf("internal error: cannot compute hash for %q: %w", path.Path, err) } actualHash := hex.EncodeToString(h) @@ -295,7 +281,7 @@ func verifyHardlinks(headInfo os.FileInfo, headPath string, rootDir string, path func getPhysicalInode(info os.FileInfo) (uint64, error) { stat, ok := info.Sys().(*syscall.Stat_t) if !ok { - return 0, fmt.Errorf("cannot get syscall stat info for %q", info.Name()) + return 0, fmt.Errorf("internal error: cannot get syscall stat info for %q", info.Name()) } return stat.Ino, nil } From df49dfceaf4d8ccbd47d0cb0e624ce638fabd87c Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 3 Dec 2025 17:01:02 +0100 Subject: [PATCH 15/54] style: respect internal best practices Signed-off-by: Paul Mars --- internal/manifestutil/verify.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index 748a8813..66d904b1 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -23,7 +23,7 @@ func VerifyDir(mfest *manifest.Manifest, rootDir string) error { } for _, group := range pathGroups { - err := verifyGroup(group, rootDir) + err = verifyGroup(group, rootDir) if err != nil { return err } @@ -67,10 +67,9 @@ func groupPaths(mfest *manifest.Manifest) ([]*pathGroup, error) { inodeToGroup[inode] = group pathGroups = append(pathGroups, group) return nil - } else { - // Add path to the existing group - group.paths = append(group.paths, path.Path) } + // Add path to the existing group + group.paths = append(group.paths, path.Path) return nil }) @@ -78,7 +77,7 @@ func groupPaths(mfest *manifest.Manifest) ([]*pathGroup, error) { return nil, err } - // sort paths + // Sort paths in groups for deterministic behavior for _, group := range pathGroups { if len(group.paths) > 1 { slices.Sort(group.paths) @@ -91,7 +90,6 @@ func groupPaths(mfest *manifest.Manifest) ([]*pathGroup, error) { // verifyGroup verifies a group of paths func verifyGroup(group *pathGroup, rootDir string) error { path := group.head - fpath := filepath.Join(rootDir, path.Path) info, err := os.Lstat(fpath) if err != nil { @@ -117,7 +115,7 @@ func verifyPath(path *manifest.Path, info os.FileInfo, fpath string) error { return err } - if strings.HasSuffix(path.Path, "/") { + if pathIsDir(path.Path) { // Directories have no additional checks return nil } @@ -127,7 +125,7 @@ func verifyPath(path *manifest.Path, info os.FileInfo, fpath string) error { } if !mode.IsRegular() { - return fmt.Errorf("tampered content: %q has unrecognized type %s.", path.Path, mode.String()) + return fmt.Errorf("tampered content: %q has unrecognized type %s", path.Path, mode.String()) } if err := verifySize(path, info); err != nil { @@ -142,7 +140,7 @@ func verifyPath(path *manifest.Path, info os.FileInfo, fpath string) error { // verifyFileType checks that the file type matches expectations. func verifyFileType(path *manifest.Path, info os.FileInfo) error { mode := info.Mode() - isDir := strings.HasSuffix(path.Path, "/") + isDir := pathIsDir(path.Path) isSymlink := path.Link != "" if isDir && !info.IsDir() { @@ -285,3 +283,7 @@ func getPhysicalInode(info os.FileInfo) (uint64, error) { } return stat.Ino, nil } + +func pathIsDir(path string) bool { + return strings.HasSuffix(path, "/") +} From f4169b98b9435f85776a837c51db48ebb09e0c86 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 4 Dec 2025 13:52:21 +0100 Subject: [PATCH 16/54] style: minor consistency fixes Signed-off-by: Paul Mars --- internal/manifestutil/verify.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index 66d904b1..a06bf1b5 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -183,7 +183,7 @@ func verifyMode(path *manifest.Path, mode os.FileMode) error { func verifySymlink(path *manifest.Path, fpath string) error { link, err := os.Readlink(fpath) if err != nil { - return fmt.Errorf("cannot read symlink %q: %w", path.Path, err) + return fmt.Errorf("internal error: cannot read symlink %q: %w", path.Path, err) } if link != path.Link { @@ -267,7 +267,7 @@ func verifyHardlinks(headInfo os.FileInfo, headPath string, rootDir string, path return err } - // Verify Inode Equality + // Verify Inode Equality. if sibInode != headInode { return fmt.Errorf("tampered content: broken hardlink: %s and %s should share inode but do not", headPath, siblingPath) } From aade4545b5f551d198e9522c235b6e8bb78b9925 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 5 Dec 2025 08:32:44 +0100 Subject: [PATCH 17/54] fix: ignore size check on manifest Signed-off-by: Paul Mars --- internal/manifestutil/manifestutil.go | 2 +- internal/manifestutil/verify.go | 44 ++++++++++++++------------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 55b55b90..00058a98 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -477,7 +477,7 @@ func hash(path string) ([]byte, error) { func SliceKeys(mfest *manifest.Manifest) []setup.SliceKey { sliceKeys := []setup.SliceKey{} mfest.IterateSlices("", func(slice *manifest.Slice) error { - sk, err := apacheutil.ParseSliceKey(slice.Name) + sk, err := setup.ParseSliceKey(slice.Name) if err != nil { return err } diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index a06bf1b5..cb73981a 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -128,13 +128,22 @@ func verifyPath(path *manifest.Path, info os.FileInfo, fpath string) error { return fmt.Errorf("tampered content: %q has unrecognized type %s", path.Path, mode.String()) } + expectedHash := recordedHash(path) + // Skip hash and size verification if no hash is declared. + // The manifest validation ensures only manifests can have + // no hash. + if expectedHash == "" { + // No hash to verify, skip size and hash checks + return nil + } + if err := verifySize(path, info); err != nil { return err } // Verify hash // Most expensive operation, so do it at the end. - return verifyHash(path, fpath) + return verifyHash(path, expectedHash, fpath) } // verifyFileType checks that the file type matches expectations. @@ -194,6 +203,16 @@ func verifySymlink(path *manifest.Path, fpath string) error { return nil } +// recordedHash returns FinalSHA256 if present, otherwise SHA256. +func recordedHash(path *manifest.Path) string { + expectedHash := path.FinalSHA256 + + if expectedHash == "" { + expectedHash = path.SHA256 + } + return expectedHash +} + // verifySize checks file size matches the manifest. func verifySize(path *manifest.Path, info os.FileInfo) error { expected := int64(path.Size) @@ -208,24 +227,7 @@ func verifySize(path *manifest.Path, info os.FileInfo) error { } // verifyHash verifies file content hash. -// Uses FinalSHA256 if present, otherwise SHA256. -// Files without any hash declaration are skipped (e.g., manifest.wall). -func verifyHash(path *manifest.Path, fpath string) error { - expectedHash := path.FinalSHA256 - hashType := "final" - - if expectedHash == "" { - expectedHash = path.SHA256 - hashType = "original" - } - - // Skip hash verification if no hash is declared - if expectedHash == "" { - // This is skipping manifest.wall that is generated - // during the cut operation and have no predetermined hash - return nil - } - +func verifyHash(path *manifest.Path, expectedHash string, fpath string) error { h, err := hash(fpath) if err != nil { return fmt.Errorf("internal error: cannot compute hash for %q: %w", path.Path, err) @@ -233,8 +235,8 @@ func verifyHash(path *manifest.Path, fpath string) error { actualHash := hex.EncodeToString(h) if actualHash != expectedHash { - return fmt.Errorf("tampered file: %q %s hash mismatch: expected %s, observed %s", - path.Path, hashType, expectedHash, actualHash) + return fmt.Errorf("tampered file: %q hash mismatch: expected %s, observed %s", + path.Path, expectedHash, actualHash) } return nil From e0bd4a4cf3920e8a920877d47302f40be89a3fe3 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 8 Dec 2025 08:21:56 +0100 Subject: [PATCH 18/54] style: apply suggestions Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 38 ++++++++++++--------------- internal/manifestutil/manifestutil.go | 36 ++++++++++++------------- internal/manifestutil/verify.go | 27 +++++++++---------- 3 files changed, 47 insertions(+), 54 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 1a3e1cdb..dec0c4da 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -76,12 +76,23 @@ func (cmd *cmdCut) Execute(args []string) error { } } - targetDir, err := nonEmptyDir(cmd.RootDir) + // Get targetDir path + // Note: This is already done in `slicer.Run`. Extract it and avoid doing it twice? + targetDir := filepath.Clean(cmd.RootDir) + if !filepath.IsAbs(targetDir) { + dir, err := os.Getwd() + if err != nil { + return fmt.Errorf("cannot obtain current directory: %w", err) + } + targetDir = filepath.Join(dir, targetDir) + } + + empty, err := emptyDir(targetDir) if err != nil { return err } - if len(targetDir) > 0 { + if !empty { manifest, err := manifestutil.FromDir(release, targetDir) if err != nil { // TODO: When enabling the feature, error out. @@ -160,27 +171,12 @@ func (cmd *cmdCut) Execute(args []string) error { return err } -// nonEmptyDir checks whether the given directory exists and is non-empty. -func nonEmptyDir(rootdir string) (string, error) { - // Get targetDir path - // Note: This is already done in `slicer.Run`. Extract it and avoid doing it twice? - targetDir := filepath.Clean(rootdir) - if !filepath.IsAbs(targetDir) { - dir, err := os.Getwd() - if err != nil { - return "", fmt.Errorf("cannot obtain current directory: %w", err) - } - targetDir = filepath.Join(dir, targetDir) - } - +// emptyDir checks whether the given directory is empty. +func emptyDir(targetDir string) (bool, error) { entries, err := os.ReadDir(targetDir) if err != nil { - return "", fmt.Errorf("cannot read root directory %q: %v", targetDir, err) - } - - if len(entries) == 0 { - return "", nil + return false, fmt.Errorf("cannot read root directory %q: %v", targetDir, err) } - return targetDir, nil + return len(entries) == 0, nil } diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 00058a98..f79a00b5 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -377,8 +377,8 @@ func FromDir(release *setup.Release, targetDir string) (*manifest.Manifest, erro // Select the first manifest of the list as the reference one for now. // Another heuristic could be used (ex. select the one from base-files_chisel). referenceRelPath := manifestPaths[0] - referencePath := path.Join(targetDir, referenceRelPath) - reference, err := load(referencePath) + referenceAbsPath := path.Join(targetDir, referenceRelPath) + reference, err := load(referenceAbsPath) if err != nil { return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", referenceRelPath, err) } @@ -420,45 +420,45 @@ func load(manifestPath string) (*manifest.Manifest, error) { // checkConsistency checks consistency between a list of manifests and a // reference one. -func checkConsistency(referenceRelPath string, targetDir string, manifests []string) error { - reference := path.Join(targetDir, referenceRelPath) - hashReference, err := hash(reference) +func checkConsistency(refRelPath string, targetDir string, manifestPaths []string) error { + ref := path.Join(targetDir, refRelPath) + refHash, err := contentHash(ref) if err != nil { - return fmt.Errorf("internal error: cannot compute hash for %q: %w", referenceRelPath, err) + return fmt.Errorf("internal error: cannot compute hash for %q: %w", refRelPath, err) } - infoRef, err := os.Stat(reference) + refInfo, err := os.Stat(ref) if err != nil { - return fmt.Errorf("internal error: cannot get file info for %q: %w", referenceRelPath, err) + return fmt.Errorf("internal error: cannot get file info for %q: %w", refRelPath, err) } - modeRef := infoRef.Mode() + refMode := refInfo.Mode() - for _, manifestRelPath := range manifests { + for _, manifestRelPath := range manifestPaths { manifestPath := path.Join(targetDir, manifestRelPath) - infoManifest, err := os.Stat(manifestPath) + manifestInfo, err := os.Stat(manifestPath) if err != nil { return fmt.Errorf("internal error: cannot get file info for %q: %w", manifestRelPath, err) } - modeManifest := infoManifest.Mode() - if modeManifest != modeRef { - return fmt.Errorf("invalid manifest: permissions on %s (%s) are different from the reference manifest %s (%s)", manifestRelPath, modeManifest, referenceRelPath, modeRef) + manifestMode := manifestInfo.Mode() + if manifestMode != refMode { + return fmt.Errorf("invalid manifest: permissions on %s (%s) are different from the reference manifest %s (%s)", manifestRelPath, manifestMode, refRelPath, refMode) } - hashM, err := hash(manifestPath) + manifestHash, err := contentHash(manifestPath) if err != nil { return fmt.Errorf("internal error: cannot compute hash for %q: %w", manifestRelPath, err) } - if !slices.Equal(hashM, hashReference) { - return fmt.Errorf("invalid manifest: %s is inconsistent with %s", manifestRelPath, referenceRelPath) + if !slices.Equal(manifestHash, refHash) { + return fmt.Errorf("invalid manifest: %s is inconsistent with %s", manifestRelPath, refRelPath) } } return nil } -func hash(path string) ([]byte, error) { +func contentHash(path string) ([]byte, error) { f, err := os.Open(path) if err != nil { return nil, err diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index cb73981a..dc57dd50 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -125,7 +125,7 @@ func verifyPath(path *manifest.Path, info os.FileInfo, fpath string) error { } if !mode.IsRegular() { - return fmt.Errorf("tampered content: %q has unrecognized type %s", path.Path, mode.String()) + return fmt.Errorf("inconsistent content: %q has unrecognized type %s", path.Path, mode.String()) } expectedHash := recordedHash(path) @@ -153,22 +153,22 @@ func verifyFileType(path *manifest.Path, info os.FileInfo) error { isSymlink := path.Link != "" if isDir && !info.IsDir() { - return fmt.Errorf("tampered content: %q expected to be a directory but found %s", + return fmt.Errorf("inconsistent content: %q expected to be a directory but found %s", path.Path, mode.Type().String()) } if !isDir && info.IsDir() { - return fmt.Errorf("tampered content: %q is a directory but manifest expects a file", + return fmt.Errorf("inconsistent content: %q is a directory but manifest expects a file", path.Path) } if isSymlink && mode.Type() != os.ModeSymlink { - return fmt.Errorf("tampered content: %q expected to be a symlink but found %s", + return fmt.Errorf("inconsistent content: %q expected to be a symlink but found %s", path.Path, mode.Type().String()) } if !isSymlink && mode.Type() == os.ModeSymlink { - return fmt.Errorf("tampered content: %q is a symlink but manifest expects regular file", + return fmt.Errorf("inconsistent content: %q is a symlink but manifest expects regular file", path.Path) } @@ -179,9 +179,8 @@ func verifyFileType(path *manifest.Path, info os.FileInfo) error { func verifyMode(path *manifest.Path, mode os.FileMode) error { expectedMode := path.Mode actualMode := fmt.Sprintf("0%o", unixPerm(mode)) - if actualMode != expectedMode { - return fmt.Errorf("tampered content: %q mode mismatch: expected %s, observed %s", + return fmt.Errorf("inconsistent content: %q mode mismatch: expected %s, observed %s", path.Path, expectedMode, actualMode) } @@ -196,17 +195,16 @@ func verifySymlink(path *manifest.Path, fpath string) error { } if link != path.Link { - return fmt.Errorf("tampered content: %q symlink mismatch: expected %q → %q, observed %q → %q", + return fmt.Errorf("inconsistent content: %q symlink mismatch: expected %q → %q, observed %q → %q", path.Path, path.Path, path.Link, path.Path, link) } return nil } -// recordedHash returns FinalSHA256 if present, otherwise SHA256. +// recordedHash returns path.FinalSHA256 if present, otherwise path.SHA256. func recordedHash(path *manifest.Path) string { expectedHash := path.FinalSHA256 - if expectedHash == "" { expectedHash = path.SHA256 } @@ -217,9 +215,8 @@ func recordedHash(path *manifest.Path) string { func verifySize(path *manifest.Path, info os.FileInfo) error { expected := int64(path.Size) actual := info.Size() - if actual != expected { - return fmt.Errorf("tampered file: %q size mismatch: expected %d bytes, observed %d bytes", + return fmt.Errorf("inconsistent file: %q size mismatch: expected %d bytes, observed %d bytes", path.Path, expected, actual) } @@ -228,14 +225,14 @@ func verifySize(path *manifest.Path, info os.FileInfo) error { // verifyHash verifies file content hash. func verifyHash(path *manifest.Path, expectedHash string, fpath string) error { - h, err := hash(fpath) + h, err := contentHash(fpath) if err != nil { return fmt.Errorf("internal error: cannot compute hash for %q: %w", path.Path, err) } actualHash := hex.EncodeToString(h) if actualHash != expectedHash { - return fmt.Errorf("tampered file: %q hash mismatch: expected %s, observed %s", + return fmt.Errorf("inconsistent file: %q hash mismatch: expected %s, observed %s", path.Path, expectedHash, actualHash) } @@ -271,7 +268,7 @@ func verifyHardlinks(headInfo os.FileInfo, headPath string, rootDir string, path // Verify Inode Equality. if sibInode != headInode { - return fmt.Errorf("tampered content: broken hardlink: %s and %s should share inode but do not", headPath, siblingPath) + return fmt.Errorf("inconsistent content: broken hardlink: %s and %s expected to share the same inode", headPath, siblingPath) } } return nil From 2e572fb32c66cc80c36ac8044a6e9f7eb42cd6c7 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 8 Dec 2025 08:31:40 +0100 Subject: [PATCH 19/54] style: apply suggestions Signed-off-by: Paul Mars --- internal/manifestutil/manifestutil.go | 2 +- internal/manifestutil/verify.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index f79a00b5..f983183c 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -443,7 +443,7 @@ func checkConsistency(refRelPath string, targetDir string, manifestPaths []strin manifestMode := manifestInfo.Mode() if manifestMode != refMode { - return fmt.Errorf("invalid manifest: permissions on %s (%s) are different from the reference manifest %s (%s)", manifestRelPath, manifestMode, refRelPath, refMode) + return fmt.Errorf("invalid manifest: permissions on %s (%s) different from %s (%s)", manifestRelPath, manifestMode, refRelPath, refMode) } manifestHash, err := contentHash(manifestPath) diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index dc57dd50..9ae7d25e 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -216,7 +216,7 @@ func verifySize(path *manifest.Path, info os.FileInfo) error { expected := int64(path.Size) actual := info.Size() if actual != expected { - return fmt.Errorf("inconsistent file: %q size mismatch: expected %d bytes, observed %d bytes", + return fmt.Errorf("inconsistent content: %q size mismatch: expected %d bytes, observed %d bytes", path.Path, expected, actual) } @@ -232,7 +232,7 @@ func verifyHash(path *manifest.Path, expectedHash string, fpath string) error { actualHash := hex.EncodeToString(h) if actualHash != expectedHash { - return fmt.Errorf("inconsistent file: %q hash mismatch: expected %s, observed %s", + return fmt.Errorf("inconsistent content: %q hash mismatch: expected %s, observed %s", path.Path, expectedHash, actualHash) } From 3c5de1cd7475cd01ee5fa16bb35c8eea1ff5b550 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 8 Dec 2025 09:03:57 +0100 Subject: [PATCH 20/54] style: apply suggestions Signed-off-by: Paul Mars --- internal/manifestutil/verify.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index 9ae7d25e..66236208 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -89,18 +89,18 @@ func groupPaths(mfest *manifest.Manifest) ([]*pathGroup, error) { // verifyGroup verifies a group of paths func verifyGroup(group *pathGroup, rootDir string) error { - path := group.head - fpath := filepath.Join(rootDir, path.Path) - info, err := os.Lstat(fpath) + head := group.head + fullPath := filepath.Join(rootDir, head.Path) + info, err := os.Lstat(fullPath) if err != nil { return err } - if err := verifyPath(path, info, fpath); err != nil { + if err := verifyPath(head, info, fullPath); err != nil { return err } - return verifyHardlinks(info, path.Path, rootDir, group.paths) + return verifyHardlinks(info, head.Path, rootDir, group.paths) } // verifyPath verifies a single path against its manifest entry @@ -246,7 +246,7 @@ func verifyHardlinks(headInfo os.FileInfo, headPath string, rootDir string, path return nil } - headInode, err := getPhysicalInode(headInfo) + headInode, err := getInode(headInfo) if err != nil { return err } @@ -261,7 +261,7 @@ func verifyHardlinks(headInfo os.FileInfo, headPath string, rootDir string, path return err } - sibInode, err := getPhysicalInode(sibFi) + sibInode, err := getInode(sibFi) if err != nil { return err } @@ -274,8 +274,8 @@ func verifyHardlinks(headInfo os.FileInfo, headPath string, rootDir string, path return nil } -// getPhysicalInode retrieves the inode number from os.FileInfo -func getPhysicalInode(info os.FileInfo) (uint64, error) { +// getInode retrieves the inode number from os.FileInfo +func getInode(info os.FileInfo) (uint64, error) { stat, ok := info.Sys().(*syscall.Stat_t) if !ok { return 0, fmt.Errorf("internal error: cannot get syscall stat info for %q", info.Name()) From 7eff973f0d8358525b3f602f26f325626bb32943 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 8 Dec 2025 09:30:23 +0100 Subject: [PATCH 21/54] style: apply suggestions Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 6 ++-- internal/manifestutil/manifestutil.go | 8 ----- internal/manifestutil/verify.go | 42 +++++++++------------------ 3 files changed, 15 insertions(+), 41 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index dec0c4da..9cbd20b8 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -87,12 +87,11 @@ func (cmd *cmdCut) Execute(args []string) error { targetDir = filepath.Join(dir, targetDir) } - empty, err := emptyDir(targetDir) + isTargetEmpty, err := emptyDir(targetDir) if err != nil { return err } - - if !empty { + if !isTargetEmpty { manifest, err := manifestutil.FromDir(release, targetDir) if err != nil { // TODO: When enabling the feature, error out. @@ -177,6 +176,5 @@ func emptyDir(targetDir string) (bool, error) { if err != nil { return false, fmt.Errorf("cannot read root directory %q: %v", targetDir, err) } - return len(entries) == 0, nil } diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index f983183c..9dbd497e 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -382,12 +382,10 @@ func FromDir(release *setup.Release, targetDir string) (*manifest.Manifest, erro if err != nil { return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", referenceRelPath, err) } - err = checkConsistency(referenceRelPath, targetDir, manifestPaths[1:]) if err != nil { return nil, err } - return reference, nil } @@ -431,16 +429,13 @@ func checkConsistency(refRelPath string, targetDir string, manifestPaths []strin if err != nil { return fmt.Errorf("internal error: cannot get file info for %q: %w", refRelPath, err) } - refMode := refInfo.Mode() - for _, manifestRelPath := range manifestPaths { manifestPath := path.Join(targetDir, manifestRelPath) manifestInfo, err := os.Stat(manifestPath) if err != nil { return fmt.Errorf("internal error: cannot get file info for %q: %w", manifestRelPath, err) } - manifestMode := manifestInfo.Mode() if manifestMode != refMode { return fmt.Errorf("invalid manifest: permissions on %s (%s) different from %s (%s)", manifestRelPath, manifestMode, refRelPath, refMode) @@ -454,7 +449,6 @@ func checkConsistency(refRelPath string, targetDir string, manifestPaths []strin return fmt.Errorf("invalid manifest: %s is inconsistent with %s", manifestRelPath, refRelPath) } } - return nil } @@ -469,7 +463,6 @@ func contentHash(path string) ([]byte, error) { if _, err := io.Copy(h, f); err != nil { return nil, err } - return h.Sum(nil), nil } @@ -484,6 +477,5 @@ func SliceKeys(mfest *manifest.Manifest) []setup.SliceKey { sliceKeys = append(sliceKeys, sk) return nil }) - return sliceKeys } diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index 66236208..d1b4a459 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -21,14 +21,12 @@ func VerifyDir(mfest *manifest.Manifest, rootDir string) error { if err != nil { return err } - for _, group := range pathGroups { err = verifyGroup(group, rootDir) if err != nil { return err } } - return nil } @@ -83,7 +81,6 @@ func groupPaths(mfest *manifest.Manifest) ([]*pathGroup, error) { slices.Sort(group.paths) } } - return pathGroups, nil } @@ -95,22 +92,19 @@ func verifyGroup(group *pathGroup, rootDir string) error { if err != nil { return err } - if err := verifyPath(head, info, fullPath); err != nil { return err } - return verifyHardlinks(info, head.Path, rootDir, group.paths) } // verifyPath verifies a single path against its manifest entry -func verifyPath(path *manifest.Path, info os.FileInfo, fpath string) error { - mode := info.Mode() - +func verifyPath(path *manifest.Path, info os.FileInfo, fullPath string) error { if err := verifyFileType(path, info); err != nil { return err } - + + mode := info.Mode() if err := verifyMode(path, mode); err != nil { return err } @@ -121,7 +115,7 @@ func verifyPath(path *manifest.Path, info os.FileInfo, fpath string) error { } if len(path.Link) > 0 { - return verifySymlink(path, fpath) + return verifySymlink(path, fullPath) } if !mode.IsRegular() { @@ -143,35 +137,32 @@ func verifyPath(path *manifest.Path, info os.FileInfo, fpath string) error { // Verify hash // Most expensive operation, so do it at the end. - return verifyHash(path, expectedHash, fpath) + return verifyHash(path, expectedHash, fullPath) } // verifyFileType checks that the file type matches expectations. func verifyFileType(path *manifest.Path, info os.FileInfo) error { mode := info.Mode() + isDir := pathIsDir(path.Path) - isSymlink := path.Link != "" - if isDir && !info.IsDir() { return fmt.Errorf("inconsistent content: %q expected to be a directory but found %s", - path.Path, mode.Type().String()) + path.Path, mode.Type().String()) } - if !isDir && info.IsDir() { return fmt.Errorf("inconsistent content: %q is a directory but manifest expects a file", - path.Path) + path.Path) } + isSymlink := path.Link != "" if isSymlink && mode.Type() != os.ModeSymlink { return fmt.Errorf("inconsistent content: %q expected to be a symlink but found %s", path.Path, mode.Type().String()) } - if !isSymlink && mode.Type() == os.ModeSymlink { return fmt.Errorf("inconsistent content: %q is a symlink but manifest expects regular file", path.Path) } - return nil } @@ -183,22 +174,19 @@ func verifyMode(path *manifest.Path, mode os.FileMode) error { return fmt.Errorf("inconsistent content: %q mode mismatch: expected %s, observed %s", path.Path, expectedMode, actualMode) } - return nil } // verifySymlink checks symlink target matches the manifest. -func verifySymlink(path *manifest.Path, fpath string) error { - link, err := os.Readlink(fpath) +func verifySymlink(path *manifest.Path, fullPath string) error { + link, err := os.Readlink(fullPath) if err != nil { return fmt.Errorf("internal error: cannot read symlink %q: %w", path.Path, err) } - if link != path.Link { return fmt.Errorf("inconsistent content: %q symlink mismatch: expected %q → %q, observed %q → %q", path.Path, path.Path, path.Link, path.Path, link) } - return nil } @@ -219,7 +207,6 @@ func verifySize(path *manifest.Path, info os.FileInfo) error { return fmt.Errorf("inconsistent content: %q size mismatch: expected %d bytes, observed %d bytes", path.Path, expected, actual) } - return nil } @@ -229,13 +216,11 @@ func verifyHash(path *manifest.Path, expectedHash string, fpath string) error { if err != nil { return fmt.Errorf("internal error: cannot compute hash for %q: %w", path.Path, err) } - actualHash := hex.EncodeToString(h) if actualHash != expectedHash { return fmt.Errorf("inconsistent content: %q hash mismatch: expected %s, observed %s", path.Path, expectedHash, actualHash) } - return nil } @@ -260,14 +245,13 @@ func verifyHardlinks(headInfo os.FileInfo, headPath string, rootDir string, path if err != nil { return err } - - sibInode, err := getInode(sibFi) + siblingInode, err := getInode(sibFi) if err != nil { return err } // Verify Inode Equality. - if sibInode != headInode { + if siblingInode != headInode { return fmt.Errorf("inconsistent content: broken hardlink: %s and %s expected to share the same inode", headPath, siblingPath) } } From b7c7c66106e00d66bcd04a67e3926f58756d6365 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 8 Dec 2025 14:51:03 +0100 Subject: [PATCH 22/54] style: apply suggestions Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 8 ++++---- internal/manifestutil/manifestutil.go | 25 +++++++++++++++---------- internal/manifestutil/verify.go | 16 ++++++---------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 9cbd20b8..dc20a9b2 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -87,11 +87,11 @@ func (cmd *cmdCut) Execute(args []string) error { targetDir = filepath.Join(dir, targetDir) } - isTargetEmpty, err := emptyDir(targetDir) + targetDirEmpty, err := dirIsEmpty(targetDir) if err != nil { return err } - if !isTargetEmpty { + if !targetDirEmpty { manifest, err := manifestutil.FromDir(release, targetDir) if err != nil { // TODO: When enabling the feature, error out. @@ -170,8 +170,8 @@ func (cmd *cmdCut) Execute(args []string) error { return err } -// emptyDir checks whether the given directory is empty. -func emptyDir(targetDir string) (bool, error) { +// dirIsEmpty checks whether the given directory is empty. +func dirIsEmpty(targetDir string) (bool, error) { entries, err := os.ReadDir(targetDir) if err != nil { return false, fmt.Errorf("cannot read root directory %q: %v", targetDir, err) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 9dbd497e..0fa68b72 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -375,18 +375,18 @@ func FromDir(release *setup.Release, targetDir string) (*manifest.Manifest, erro } // Select the first manifest of the list as the reference one for now. - // Another heuristic could be used (ex. select the one from base-files_chisel). - referenceRelPath := manifestPaths[0] - referenceAbsPath := path.Join(targetDir, referenceRelPath) - reference, err := load(referenceAbsPath) + manifestPath := manifestPaths[0] + manifest, err := load(path.Join(targetDir, manifestPath)) if err != nil { - return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", referenceRelPath, err) + return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", manifestPath, err) } - err = checkConsistency(referenceRelPath, targetDir, manifestPaths[1:]) + + err = checkIdentical(targetDir, manifestPaths) if err != nil { return nil, err } - return reference, nil + + return manifest, nil } // load reads, validates and returns a manifest. @@ -416,9 +416,14 @@ func load(manifestPath string) (*manifest.Manifest, error) { return mfest, nil } -// checkConsistency checks consistency between a list of manifests and a -// reference one. -func checkConsistency(refRelPath string, targetDir string, manifestPaths []string) error { +// checkIdentical checks all manifests in the passed list are identical +// to the reference one. +func checkIdentical(targetDir string, manifestPaths []string) error { + if len(manifestPaths) == 0 { + return nil + } + refRelPath := manifestPaths[0] + ref := path.Join(targetDir, refRelPath) refHash, err := contentHash(ref) if err != nil { diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index d1b4a459..6f7e54a1 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -103,13 +103,13 @@ func verifyPath(path *manifest.Path, info os.FileInfo, fullPath string) error { if err := verifyFileType(path, info); err != nil { return err } - + mode := info.Mode() if err := verifyMode(path, mode); err != nil { return err } - if pathIsDir(path.Path) { + if strings.HasSuffix(path.Path, "/") { // Directories have no additional checks return nil } @@ -143,15 +143,15 @@ func verifyPath(path *manifest.Path, info os.FileInfo, fullPath string) error { // verifyFileType checks that the file type matches expectations. func verifyFileType(path *manifest.Path, info os.FileInfo) error { mode := info.Mode() - - isDir := pathIsDir(path.Path) + + isDir := strings.HasSuffix(path.Path, "/") if isDir && !info.IsDir() { return fmt.Errorf("inconsistent content: %q expected to be a directory but found %s", - path.Path, mode.Type().String()) + path.Path, mode.Type().String()) } if !isDir && info.IsDir() { return fmt.Errorf("inconsistent content: %q is a directory but manifest expects a file", - path.Path) + path.Path) } isSymlink := path.Link != "" @@ -266,7 +266,3 @@ func getInode(info os.FileInfo) (uint64, error) { } return stat.Ino, nil } - -func pathIsDir(path string) bool { - return strings.HasSuffix(path, "/") -} From c7196e9928c45d81ff8c11cf038a20eec5235c2b Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 16 Dec 2025 08:43:32 +0100 Subject: [PATCH 23/54] fix: apply suggestions Signed-off-by: Paul Mars --- cmd/chisel/cmd_cut.go | 2 +- internal/manifestutil/manifestutil.go | 3 -- internal/manifestutil/verify.go | 67 ++++++++++++--------------- 3 files changed, 31 insertions(+), 41 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index dc20a9b2..717593af 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -97,7 +97,7 @@ func (cmd *cmdCut) Execute(args []string) error { // TODO: When enabling the feature, error out. logf("Warning: %v", err) } else { - err = manifestutil.VerifyDir(manifest, targetDir) + err = manifestutil.CheckDir(manifest, targetDir) if err != nil { // TODO: When enabling the feature, error out. logf("Warning: %v", err) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 0fa68b72..fb19a722 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -47,14 +47,11 @@ func FindPathsInRelease(r *setup.Release) []string { allSlices = append(allSlices, slice) } } - manifestMap := FindPaths(allSlices) - paths := make([]string, 0, len(manifestMap)) for path := range manifestMap { paths = append(paths, path) } - return paths } diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/verify.go index 6f7e54a1..6b0dbbab 100644 --- a/internal/manifestutil/verify.go +++ b/internal/manifestutil/verify.go @@ -12,17 +12,17 @@ import ( "github.com/canonical/chisel/public/manifest" ) -// VerifyDir verifies the content of the target directory matches with +// CheckDir checks the content of the target directory matches with // the manifest. // This function works under the assumption the manifest is valid. // Files not managed by chisel are ignored. -func VerifyDir(mfest *manifest.Manifest, rootDir string) error { - pathGroups, err := groupPaths(mfest) +func CheckDir(mfest *manifest.Manifest, rootDir string) error { + pathGroups, err := pathGroups(mfest) if err != nil { return err } for _, group := range pathGroups { - err = verifyGroup(group, rootDir) + err = checkGroup(group, rootDir) if err != nil { return err } @@ -35,19 +35,19 @@ type pathGroup struct { paths []string } -// groupPaths groups paths by inode. +// pathGroups groups paths by inode. // Returns a slice of path groups where each group represents either: // - A standalone file (inode 0) // - A set of hardlinked files (same non-zero inode) -func groupPaths(mfest *manifest.Manifest) ([]*pathGroup, error) { - pathGroups := []*pathGroup{} +func pathGroups(mfest *manifest.Manifest) ([]*pathGroup, error) { + groups := []*pathGroup{} inodeToGroup := make(map[uint64]*pathGroup) err := mfest.IteratePaths("", func(path *manifest.Path) error { inode := path.Inode if inode == 0 { // Standalone path - pathGroups = append(pathGroups, &pathGroup{ + groups = append(groups, &pathGroup{ head: path, paths: []string{path.Path}, }) @@ -63,7 +63,7 @@ func groupPaths(mfest *manifest.Manifest) ([]*pathGroup, error) { paths: []string{path.Path}, } inodeToGroup[inode] = group - pathGroups = append(pathGroups, group) + groups = append(groups, group) return nil } // Add path to the existing group @@ -76,36 +76,34 @@ func groupPaths(mfest *manifest.Manifest) ([]*pathGroup, error) { } // Sort paths in groups for deterministic behavior - for _, group := range pathGroups { + for _, group := range groups { if len(group.paths) > 1 { slices.Sort(group.paths) } } - return pathGroups, nil + return groups, nil } -// verifyGroup verifies a group of paths -func verifyGroup(group *pathGroup, rootDir string) error { +func checkGroup(group *pathGroup, rootDir string) error { head := group.head fullPath := filepath.Join(rootDir, head.Path) info, err := os.Lstat(fullPath) if err != nil { return err } - if err := verifyPath(head, info, fullPath); err != nil { + if err := checkPath(head, info, fullPath); err != nil { return err } - return verifyHardlinks(info, head.Path, rootDir, group.paths) + return checkHardlinks(info, head.Path, rootDir, group.paths) } -// verifyPath verifies a single path against its manifest entry -func verifyPath(path *manifest.Path, info os.FileInfo, fullPath string) error { - if err := verifyFileType(path, info); err != nil { +func checkPath(path *manifest.Path, info os.FileInfo, fullPath string) error { + if err := checkFileType(path, info); err != nil { return err } mode := info.Mode() - if err := verifyMode(path, mode); err != nil { + if err := checkMode(path, mode); err != nil { return err } @@ -115,7 +113,7 @@ func verifyPath(path *manifest.Path, info os.FileInfo, fullPath string) error { } if len(path.Link) > 0 { - return verifySymlink(path, fullPath) + return checkSymlink(path, fullPath) } if !mode.IsRegular() { @@ -131,25 +129,24 @@ func verifyPath(path *manifest.Path, info os.FileInfo, fullPath string) error { return nil } - if err := verifySize(path, info); err != nil { + if err := checkSize(path, info); err != nil { return err } // Verify hash // Most expensive operation, so do it at the end. - return verifyHash(path, expectedHash, fullPath) + return checkHash(path, expectedHash, fullPath) } -// verifyFileType checks that the file type matches expectations. -func verifyFileType(path *manifest.Path, info os.FileInfo) error { +func checkFileType(path *manifest.Path, info os.FileInfo) error { mode := info.Mode() - isDir := strings.HasSuffix(path.Path, "/") - if isDir && !info.IsDir() { + pathIsDir := strings.HasSuffix(path.Path, "/") + if pathIsDir && !info.IsDir() { return fmt.Errorf("inconsistent content: %q expected to be a directory but found %s", path.Path, mode.Type().String()) } - if !isDir && info.IsDir() { + if !pathIsDir && info.IsDir() { return fmt.Errorf("inconsistent content: %q is a directory but manifest expects a file", path.Path) } @@ -166,8 +163,7 @@ func verifyFileType(path *manifest.Path, info os.FileInfo) error { return nil } -// verifyMode checks file permissions match the manifest. -func verifyMode(path *manifest.Path, mode os.FileMode) error { +func checkMode(path *manifest.Path, mode os.FileMode) error { expectedMode := path.Mode actualMode := fmt.Sprintf("0%o", unixPerm(mode)) if actualMode != expectedMode { @@ -177,8 +173,7 @@ func verifyMode(path *manifest.Path, mode os.FileMode) error { return nil } -// verifySymlink checks symlink target matches the manifest. -func verifySymlink(path *manifest.Path, fullPath string) error { +func checkSymlink(path *manifest.Path, fullPath string) error { link, err := os.Readlink(fullPath) if err != nil { return fmt.Errorf("internal error: cannot read symlink %q: %w", path.Path, err) @@ -199,8 +194,7 @@ func recordedHash(path *manifest.Path) string { return expectedHash } -// verifySize checks file size matches the manifest. -func verifySize(path *manifest.Path, info os.FileInfo) error { +func checkSize(path *manifest.Path, info os.FileInfo) error { expected := int64(path.Size) actual := info.Size() if actual != expected { @@ -210,8 +204,7 @@ func verifySize(path *manifest.Path, info os.FileInfo) error { return nil } -// verifyHash verifies file content hash. -func verifyHash(path *manifest.Path, expectedHash string, fpath string) error { +func checkHash(path *manifest.Path, expectedHash string, fpath string) error { h, err := contentHash(fpath) if err != nil { return fmt.Errorf("internal error: cannot compute hash for %q: %w", path.Path, err) @@ -224,8 +217,8 @@ func verifyHash(path *manifest.Path, expectedHash string, fpath string) error { return nil } -// verifyHardlinks verifies that all paths in the list share the same inode. -func verifyHardlinks(headInfo os.FileInfo, headPath string, rootDir string, paths []string) error { +// checkHardlinks verifies that all paths in the list share the same inode. +func checkHardlinks(headInfo os.FileInfo, headPath string, rootDir string, paths []string) error { if len(paths) == 0 { // No hardlinks, nothing to do return nil From 7539db72341fad3965e1e172ae4d9ddee2fd28aa Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 19 Dec 2025 09:51:09 +0100 Subject: [PATCH 24/54] refactor: simplify --- cmd/chisel/cmd_cut.go | 52 ++++--------------- internal/manifestutil/{verify.go => check.go} | 0 internal/manifestutil/manifestutil.go | 21 ++++++-- 3 files changed, 29 insertions(+), 44 deletions(-) rename internal/manifestutil/{verify.go => check.go} (100%) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 717593af..b147e683 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -2,8 +2,6 @@ package main import ( "fmt" - "os" - "path/filepath" "slices" "time" @@ -76,40 +74,21 @@ func (cmd *cmdCut) Execute(args []string) error { } } - // Get targetDir path - // Note: This is already done in `slicer.Run`. Extract it and avoid doing it twice? - targetDir := filepath.Clean(cmd.RootDir) - if !filepath.IsAbs(targetDir) { - dir, err := os.Getwd() - if err != nil { - return fmt.Errorf("cannot obtain current directory: %w", err) - } - targetDir = filepath.Join(dir, targetDir) - } - - targetDirEmpty, err := dirIsEmpty(targetDir) + manifest, err := manifestutil.FromDir(release, cmd.RootDir) if err != nil { - return err - } - if !targetDirEmpty { - manifest, err := manifestutil.FromDir(release, targetDir) + // TODO: When enabling the feature, error out. + logf("Warning: %v", err) + } else { + err = manifestutil.CheckDir(manifest, cmd.RootDir) if err != nil { // TODO: When enabling the feature, error out. logf("Warning: %v", err) - } else { - err = manifestutil.CheckDir(manifest, targetDir) - if err != nil { - // TODO: When enabling the feature, error out. - logf("Warning: %v", err) - } - // Merge the slice keys used to build the existing rootfs with the ones - // explicitly requested slices. - // Previous slice keys contain both explicitly requested slices and slices - // resolved following essentials. - for _, s := range manifestutil.SliceKeys(manifest) { - if !slices.Contains(sliceKeys, s) { - sliceKeys = append(sliceKeys, s) - } + } + // Merge the slice keys used to build the existing rootfs with the ones + // explicitly requested slices. + for _, s := range manifestutil.SliceKeys(manifest) { + if !slices.Contains(sliceKeys, s) { + sliceKeys = append(sliceKeys, s) } } } @@ -169,12 +148,3 @@ func (cmd *cmdCut) Execute(args []string) error { }) return err } - -// dirIsEmpty checks whether the given directory is empty. -func dirIsEmpty(targetDir string) (bool, error) { - entries, err := os.ReadDir(targetDir) - if err != nil { - return false, fmt.Errorf("cannot read root directory %q: %v", targetDir, err) - } - return len(entries) == 0, nil -} diff --git a/internal/manifestutil/verify.go b/internal/manifestutil/check.go similarity index 100% rename from internal/manifestutil/verify.go rename to internal/manifestutil/check.go diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index fb19a722..3769e13b 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -362,15 +362,30 @@ func Validate(mfest *manifest.Manifest) (err error) { return nil } -// FromDir extracts, validates and returns the manifest from a targetDir -func FromDir(release *setup.Release, targetDir string) (*manifest.Manifest, error) { +// FromDir extracts, validates and returns the manifest from a rootDir +func FromDir(release *setup.Release, rootDir string) (*manifest.Manifest, error) { + targetDir := filepath.Clean(rootDir) + if !filepath.IsAbs(targetDir) { + dir, err := os.Getwd() + if err != nil { + return nil, fmt.Errorf("cannot obtain current directory: %w", err) + } + targetDir = filepath.Join(dir, targetDir) + } + entries, err := os.ReadDir(targetDir) + if err != nil { + return nil, fmt.Errorf("cannot read root directory %q: %v", targetDir, err) + } + if len(entries) == 0 { + return nil, fmt.Errorf("no manifest generated for this release") + } + manifestPaths := FindPathsInRelease(release) if len(manifestPaths) == 0 { // No manifest in the release means it cannot produce a rootfs that can // be recut. Treat this case as cutting a new rootfs. return nil, fmt.Errorf("no manifest generated for this release") } - // Select the first manifest of the list as the reference one for now. manifestPath := manifestPaths[0] manifest, err := load(path.Join(targetDir, manifestPath)) From cd22c1fecd97d103b399c5b051728ada01446097 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 5 Jan 2026 11:40:37 +0100 Subject: [PATCH 25/54] feat: intermediary representation --- cmd/chisel/cmd_cut.go | 2 +- internal/manifestutil/check.go | 95 ++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index b147e683..db1265c3 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -79,7 +79,7 @@ func (cmd *cmdCut) Execute(args []string) error { // TODO: When enabling the feature, error out. logf("Warning: %v", err) } else { - err = manifestutil.CheckDir(manifest, cmd.RootDir) + err = manifestutil.CheckDir2(manifest, cmd.RootDir) if err != nil { // TODO: When enabling the feature, error out. logf("Warning: %v", err) diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go index 6b0dbbab..29a63251 100644 --- a/internal/manifestutil/check.go +++ b/internal/manifestutil/check.go @@ -3,8 +3,10 @@ package manifestutil import ( "encoding/hex" "fmt" + "io/fs" "os" "path/filepath" + "reflect" "slices" "strings" "syscall" @@ -12,6 +14,99 @@ import ( "github.com/canonical/chisel/public/manifest" ) +type pathInfo struct { + mode string + size int64 + link string + hash string + inode uint64 +} + +func collectInfo(fullPath string) (*pathInfo, error) { + var err error + var link string + var hash string + var size int64 + var inode uint64 + info, err := os.Lstat(fullPath) + if err != nil { + return nil, err + } + mode := info.Mode() + ftype := mode & fs.ModeType + switch ftype { + case fs.ModeDir: + // Nothing to do + case fs.ModeSymlink: + link, err = os.Readlink(fullPath) + if err != nil { + return nil, fmt.Errorf("internal error: cannot read symlink %q: %w", fullPath, err) + } + case 0: // Regular + h, err := contentHash(fullPath) + if err != nil { + return nil, fmt.Errorf("internal error: cannot compute hash for %q: %w", fullPath, err) + } + hash = hex.EncodeToString(h) + size = info.Size() + default: + return nil, fmt.Errorf("inconsistent content: %q has unrecognized type %s", fullPath, mode.String()) + } + if ftype != fs.ModeDir { + inode, err = getInode(info) + if err != nil { + return nil, err + } + } + return &pathInfo{ + mode: fmt.Sprintf("0%o", unixPerm(mode)), + size: size, + link: link, + hash: hash, + inode: inode, + }, nil +} + +// CheckDir checks the content of the target directory matches with +// the manifest. +// This function works under the assumption the manifest is valid. +// Files not managed by chisel are ignored. +func CheckDir2(mfest *manifest.Manifest, rootDir string) error { + var inodes []uint64 + pathsByInodes := make(map[uint64][]string) + err := mfest.IteratePaths("", func(path *manifest.Path) error { + fullPath := filepath.Join(rootDir, path.Path) + collected, err := collectInfo(fullPath) + if err != nil { + return err + } + + if collected.inode != 0 { + inode := collected.inode + if len(pathsByInodes[inode]) == 1 { + inodes = append(inodes, inode) + } + pathsByInodes[inode] = append(pathsByInodes[inode], path.Path) + } + expected := &pathInfo{ + mode: path.Mode, + size: int64(path.Size), + link: path.Link, + // inode: path.Inode, + hash: recordedHash(path), + } + // Exclude manifest files + if filepath.Base(path.Path) == DefaultFilename { + return nil + } + if !reflect.DeepEqual(collected, expected) { + return fmt.Errorf("inconsistent content at %q: expected %+v, observed %+v", path.Path, expected, collected) + } + return nil + }) + return err +} + // CheckDir checks the content of the target directory matches with // the manifest. // This function works under the assumption the manifest is valid. From 27d864504fda0f9dfc4f14057fe8dbe446ce93d2 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 5 Jan 2026 14:12:32 +0100 Subject: [PATCH 26/54] refactor: simplify existing dir checking --- cmd/chisel/cmd_cut.go | 2 +- internal/manifestutil/check.go | 369 ++++++--------------------------- 2 files changed, 62 insertions(+), 309 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index db1265c3..b147e683 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -79,7 +79,7 @@ func (cmd *cmdCut) Execute(args []string) error { // TODO: When enabling the feature, error out. logf("Warning: %v", err) } else { - err = manifestutil.CheckDir2(manifest, cmd.RootDir) + err = manifestutil.CheckDir(manifest, cmd.RootDir) if err != nil { // TODO: When enabling the feature, error out. logf("Warning: %v", err) diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go index 29a63251..17b944db 100644 --- a/internal/manifestutil/check.go +++ b/internal/manifestutil/check.go @@ -7,277 +7,96 @@ import ( "os" "path/filepath" "reflect" - "slices" - "strings" "syscall" "github.com/canonical/chisel/public/manifest" ) type pathInfo struct { - mode string - size int64 - link string - hash string - inode uint64 -} - -func collectInfo(fullPath string) (*pathInfo, error) { - var err error - var link string - var hash string - var size int64 - var inode uint64 - info, err := os.Lstat(fullPath) - if err != nil { - return nil, err - } - mode := info.Mode() - ftype := mode & fs.ModeType - switch ftype { - case fs.ModeDir: - // Nothing to do - case fs.ModeSymlink: - link, err = os.Readlink(fullPath) - if err != nil { - return nil, fmt.Errorf("internal error: cannot read symlink %q: %w", fullPath, err) - } - case 0: // Regular - h, err := contentHash(fullPath) - if err != nil { - return nil, fmt.Errorf("internal error: cannot compute hash for %q: %w", fullPath, err) - } - hash = hex.EncodeToString(h) - size = info.Size() - default: - return nil, fmt.Errorf("inconsistent content: %q has unrecognized type %s", fullPath, mode.String()) - } - if ftype != fs.ModeDir { - inode, err = getInode(info) - if err != nil { - return nil, err - } - } - return &pathInfo{ - mode: fmt.Sprintf("0%o", unixPerm(mode)), - size: size, - link: link, - hash: hash, - inode: inode, - }, nil + mode string + size int64 + link string + hash string } // CheckDir checks the content of the target directory matches with // the manifest. // This function works under the assumption the manifest is valid. // Files not managed by chisel are ignored. -func CheckDir2(mfest *manifest.Manifest, rootDir string) error { - var inodes []uint64 - pathsByInodes := make(map[uint64][]string) +func CheckDir(mfest *manifest.Manifest, rootDir string) error { + mfestInodeToFSInode := make(map[uint64]uint64) err := mfest.IteratePaths("", func(path *manifest.Path) error { - fullPath := filepath.Join(rootDir, path.Path) - collected, err := collectInfo(fullPath) - if err != nil { - return err - } - - if collected.inode != 0 { - inode := collected.inode - if len(pathsByInodes[inode]) == 1 { - inodes = append(inodes, inode) - } - pathsByInodes[inode] = append(pathsByInodes[inode], path.Path) + // Skip manifest files + if filepath.Base(path.Path) == DefaultFilename { + return nil } - expected := &pathInfo{ + mfestPathInfo := &pathInfo{ mode: path.Mode, size: int64(path.Size), link: path.Link, - // inode: path.Inode, hash: recordedHash(path), } - // Exclude manifest files - if filepath.Base(path.Path) == DefaultFilename { - return nil - } - if !reflect.DeepEqual(collected, expected) { - return fmt.Errorf("inconsistent content at %q: expected %+v, observed %+v", path.Path, expected, collected) - } - return nil - }) - return err -} -// CheckDir checks the content of the target directory matches with -// the manifest. -// This function works under the assumption the manifest is valid. -// Files not managed by chisel are ignored. -func CheckDir(mfest *manifest.Manifest, rootDir string) error { - pathGroups, err := pathGroups(mfest) - if err != nil { - return err - } - for _, group := range pathGroups { - err = checkGroup(group, rootDir) + var link string + var hash string + var size int64 + var inode uint64 + fullPath := filepath.Join(rootDir, path.Path) + info, err := os.Lstat(fullPath) if err != nil { return err } - } - return nil -} - -type pathGroup struct { - head *manifest.Path - paths []string -} - -// pathGroups groups paths by inode. -// Returns a slice of path groups where each group represents either: -// - A standalone file (inode 0) -// - A set of hardlinked files (same non-zero inode) -func pathGroups(mfest *manifest.Manifest) ([]*pathGroup, error) { - groups := []*pathGroup{} - inodeToGroup := make(map[uint64]*pathGroup) - - err := mfest.IteratePaths("", func(path *manifest.Path) error { - inode := path.Inode - if inode == 0 { - // Standalone path - groups = append(groups, &pathGroup{ - head: path, - paths: []string{path.Path}, - }) - return nil + mode := info.Mode() + ftype := mode & fs.ModeType + switch ftype { + case fs.ModeDir: + // Nothing to do + case fs.ModeSymlink: + link, err = os.Readlink(fullPath) + if err != nil { + return fmt.Errorf("internal error: cannot read symlink %q: %w", fullPath, err) + } + case 0: // Regular + h, err := contentHash(fullPath) + if err != nil { + return fmt.Errorf("internal error: cannot compute hash for %q: %w", fullPath, err) + } + hash = hex.EncodeToString(h) + size = info.Size() + default: + return fmt.Errorf("inconsistent content: %q has unrecognized type %s", fullPath, mode.String()) + } + if ftype != fs.ModeDir { + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return fmt.Errorf("internal error: cannot get syscall stat info for %q", info.Name()) + } + inode = stat.Ino + } + fsEntryInfo := &pathInfo{ + mode: fmt.Sprintf("0%o", unixPerm(mode)), + size: size, + link: link, + hash: hash, } - // Hardlinked path - group, ok := inodeToGroup[inode] - if !ok { - // New group of hardlinks - group = &pathGroup{ - head: path, - paths: []string{path.Path}, + if !reflect.DeepEqual(mfestPathInfo, fsEntryInfo) { + return fmt.Errorf("inconsistent content at %q: recorded %+v, observed %+v", path.Path, mfestPathInfo, fsEntryInfo) + } + if path.Inode != 0 { + if inode == 0 { + return fmt.Errorf("inconsistent content at %q: expected hardlinks", path.Path) + } + recordedInode, ok := mfestInodeToFSInode[path.Inode] + if !ok { + mfestInodeToFSInode[path.Inode] = inode + } else if recordedInode != inode { + return fmt.Errorf("inconsistent content at %q: file hardlinked to a different inode", path.Path) } - inodeToGroup[inode] = group - groups = append(groups, group) - return nil } - // Add path to the existing group - group.paths = append(group.paths, path.Path) - return nil }) - if err != nil { - return nil, err - } - - // Sort paths in groups for deterministic behavior - for _, group := range groups { - if len(group.paths) > 1 { - slices.Sort(group.paths) - } - } - return groups, nil -} - -func checkGroup(group *pathGroup, rootDir string) error { - head := group.head - fullPath := filepath.Join(rootDir, head.Path) - info, err := os.Lstat(fullPath) - if err != nil { - return err - } - if err := checkPath(head, info, fullPath); err != nil { - return err - } - return checkHardlinks(info, head.Path, rootDir, group.paths) -} - -func checkPath(path *manifest.Path, info os.FileInfo, fullPath string) error { - if err := checkFileType(path, info); err != nil { - return err - } - - mode := info.Mode() - if err := checkMode(path, mode); err != nil { - return err - } - - if strings.HasSuffix(path.Path, "/") { - // Directories have no additional checks - return nil - } - - if len(path.Link) > 0 { - return checkSymlink(path, fullPath) - } - - if !mode.IsRegular() { - return fmt.Errorf("inconsistent content: %q has unrecognized type %s", path.Path, mode.String()) - } - - expectedHash := recordedHash(path) - // Skip hash and size verification if no hash is declared. - // The manifest validation ensures only manifests can have - // no hash. - if expectedHash == "" { - // No hash to verify, skip size and hash checks - return nil - } - - if err := checkSize(path, info); err != nil { - return err - } - - // Verify hash - // Most expensive operation, so do it at the end. - return checkHash(path, expectedHash, fullPath) -} - -func checkFileType(path *manifest.Path, info os.FileInfo) error { - mode := info.Mode() - - pathIsDir := strings.HasSuffix(path.Path, "/") - if pathIsDir && !info.IsDir() { - return fmt.Errorf("inconsistent content: %q expected to be a directory but found %s", - path.Path, mode.Type().String()) - } - if !pathIsDir && info.IsDir() { - return fmt.Errorf("inconsistent content: %q is a directory but manifest expects a file", - path.Path) - } - - isSymlink := path.Link != "" - if isSymlink && mode.Type() != os.ModeSymlink { - return fmt.Errorf("inconsistent content: %q expected to be a symlink but found %s", - path.Path, mode.Type().String()) - } - if !isSymlink && mode.Type() == os.ModeSymlink { - return fmt.Errorf("inconsistent content: %q is a symlink but manifest expects regular file", - path.Path) - } - return nil -} - -func checkMode(path *manifest.Path, mode os.FileMode) error { - expectedMode := path.Mode - actualMode := fmt.Sprintf("0%o", unixPerm(mode)) - if actualMode != expectedMode { - return fmt.Errorf("inconsistent content: %q mode mismatch: expected %s, observed %s", - path.Path, expectedMode, actualMode) - } - return nil -} - -func checkSymlink(path *manifest.Path, fullPath string) error { - link, err := os.Readlink(fullPath) - if err != nil { - return fmt.Errorf("internal error: cannot read symlink %q: %w", path.Path, err) - } - if link != path.Link { - return fmt.Errorf("inconsistent content: %q symlink mismatch: expected %q → %q, observed %q → %q", - path.Path, path.Path, path.Link, path.Path, link) - } - return nil + return err } // recordedHash returns path.FinalSHA256 if present, otherwise path.SHA256. @@ -288,69 +107,3 @@ func recordedHash(path *manifest.Path) string { } return expectedHash } - -func checkSize(path *manifest.Path, info os.FileInfo) error { - expected := int64(path.Size) - actual := info.Size() - if actual != expected { - return fmt.Errorf("inconsistent content: %q size mismatch: expected %d bytes, observed %d bytes", - path.Path, expected, actual) - } - return nil -} - -func checkHash(path *manifest.Path, expectedHash string, fpath string) error { - h, err := contentHash(fpath) - if err != nil { - return fmt.Errorf("internal error: cannot compute hash for %q: %w", path.Path, err) - } - actualHash := hex.EncodeToString(h) - if actualHash != expectedHash { - return fmt.Errorf("inconsistent content: %q hash mismatch: expected %s, observed %s", - path.Path, expectedHash, actualHash) - } - return nil -} - -// checkHardlinks verifies that all paths in the list share the same inode. -func checkHardlinks(headInfo os.FileInfo, headPath string, rootDir string, paths []string) error { - if len(paths) == 0 { - // No hardlinks, nothing to do - return nil - } - - headInode, err := getInode(headInfo) - if err != nil { - return err - } - - for _, siblingPath := range paths { - if siblingPath == headPath { - continue - } - siblingFullPath := filepath.Join(rootDir, siblingPath) - sibFi, err := os.Lstat(siblingFullPath) - if err != nil { - return err - } - siblingInode, err := getInode(sibFi) - if err != nil { - return err - } - - // Verify Inode Equality. - if siblingInode != headInode { - return fmt.Errorf("inconsistent content: broken hardlink: %s and %s expected to share the same inode", headPath, siblingPath) - } - } - return nil -} - -// getInode retrieves the inode number from os.FileInfo -func getInode(info os.FileInfo) (uint64, error) { - stat, ok := info.Sys().(*syscall.Stat_t) - if !ok { - return 0, fmt.Errorf("internal error: cannot get syscall stat info for %q", info.Name()) - } - return stat.Ino, nil -} From 373f3c1b65b8e3a4ffcc2923b0e87ae3963608c8 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 5 Jan 2026 14:39:32 +0100 Subject: [PATCH 27/54] refactor: regroup hardlink-related code --- internal/manifestutil/check.go | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go index 17b944db..bb5b4612 100644 --- a/internal/manifestutil/check.go +++ b/internal/manifestutil/check.go @@ -66,26 +66,24 @@ func CheckDir(mfest *manifest.Manifest, rootDir string) error { default: return fmt.Errorf("inconsistent content: %q has unrecognized type %s", fullPath, mode.String()) } - if ftype != fs.ModeDir { - stat, ok := info.Sys().(*syscall.Stat_t) - if !ok { - return fmt.Errorf("internal error: cannot get syscall stat info for %q", info.Name()) - } - inode = stat.Ino - } fsEntryInfo := &pathInfo{ mode: fmt.Sprintf("0%o", unixPerm(mode)), size: size, link: link, hash: hash, } - if !reflect.DeepEqual(mfestPathInfo, fsEntryInfo) { return fmt.Errorf("inconsistent content at %q: recorded %+v, observed %+v", path.Path, mfestPathInfo, fsEntryInfo) } + + // Check hardlink if path.Inode != 0 { - if inode == 0 { - return fmt.Errorf("inconsistent content at %q: expected hardlinks", path.Path) + if ftype != fs.ModeDir { + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return fmt.Errorf("internal error: cannot get syscall stat info for %q", info.Name()) + } + inode = stat.Ino } recordedInode, ok := mfestInodeToFSInode[path.Inode] if !ok { From 14149907513450fa9559619dac0a673ada51e875 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 5 Jan 2026 16:52:03 +0100 Subject: [PATCH 28/54] refactor: refine manifest checking API --- cmd/chisel/cmd_cut.go | 30 ++++++++++++++------------- internal/manifestutil/manifestutil.go | 24 +++------------------ 2 files changed, 19 insertions(+), 35 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index b147e683..ee17a528 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -74,25 +74,27 @@ func (cmd *cmdCut) Execute(args []string) error { } } - manifest, err := manifestutil.FromDir(release, cmd.RootDir) - if err != nil { - // TODO: When enabling the feature, error out. - logf("Warning: %v", err) - } else { - err = manifestutil.CheckDir(manifest, cmd.RootDir) + manifestPaths := manifestutil.FindPathsInRelease(release) + if len(manifestPaths) > 0 { + manifest, err := manifestutil.FromDir(manifestPaths, cmd.RootDir) if err != nil { - // TODO: When enabling the feature, error out. logf("Warning: %v", err) - } - // Merge the slice keys used to build the existing rootfs with the ones - // explicitly requested slices. - for _, s := range manifestutil.SliceKeys(manifest) { - if !slices.Contains(sliceKeys, s) { - sliceKeys = append(sliceKeys, s) + } else { + err = manifestutil.CheckDir(manifest, cmd.RootDir) + if err != nil { + logf("Warning: %v", err) + } else { + // Merge the slice keys used to build the existing rootfs with the ones + // explicitly requested. + for _, s := range manifestutil.SliceKeys(manifest) { + if !slices.Contains(sliceKeys, s) { + sliceKeys = append(sliceKeys, s) + } + } } } } - + selection, err := setup.Select(release, sliceKeys, cmd.Arch) if err != nil { return err diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 3769e13b..08746f97 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -362,8 +362,8 @@ func Validate(mfest *manifest.Manifest) (err error) { return nil } -// FromDir extracts, validates and returns the manifest from a rootDir -func FromDir(release *setup.Release, rootDir string) (*manifest.Manifest, error) { +// FromDir extracts, checks and returns a manifest from a rootDir +func FromDir(manifestPaths []string, rootDir string) (*manifest.Manifest, error) { targetDir := filepath.Clean(rootDir) if !filepath.IsAbs(targetDir) { dir, err := os.Getwd() @@ -372,32 +372,15 @@ func FromDir(release *setup.Release, rootDir string) (*manifest.Manifest, error) } targetDir = filepath.Join(dir, targetDir) } - entries, err := os.ReadDir(targetDir) - if err != nil { - return nil, fmt.Errorf("cannot read root directory %q: %v", targetDir, err) - } - if len(entries) == 0 { - return nil, fmt.Errorf("no manifest generated for this release") - } - - manifestPaths := FindPathsInRelease(release) - if len(manifestPaths) == 0 { - // No manifest in the release means it cannot produce a rootfs that can - // be recut. Treat this case as cutting a new rootfs. - return nil, fmt.Errorf("no manifest generated for this release") - } - // Select the first manifest of the list as the reference one for now. manifestPath := manifestPaths[0] manifest, err := load(path.Join(targetDir, manifestPath)) if err != nil { return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", manifestPath, err) } - err = checkIdentical(targetDir, manifestPaths) if err != nil { return nil, err } - return manifest, nil } @@ -428,8 +411,7 @@ func load(manifestPath string) (*manifest.Manifest, error) { return mfest, nil } -// checkIdentical checks all manifests in the passed list are identical -// to the reference one. +// checkIdentical checks all manifests are identical. func checkIdentical(targetDir string, manifestPaths []string) error { if len(manifestPaths) == 0 { return nil From 2ce5b215193a7573d00725fbe658b671763b12e4 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 6 Jan 2026 08:52:12 +0100 Subject: [PATCH 29/54] refactor: add obtainManifest --- cmd/chisel/cmd_cut.go | 29 +++++++++------------------ cmd/chisel/helpers.go | 20 ++++++++++++++++++ internal/manifestutil/manifestutil.go | 3 +++ 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index ee17a528..97c301ae 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -74,27 +74,18 @@ func (cmd *cmdCut) Execute(args []string) error { } } - manifestPaths := manifestutil.FindPathsInRelease(release) - if len(manifestPaths) > 0 { - manifest, err := manifestutil.FromDir(manifestPaths, cmd.RootDir) - if err != nil { - logf("Warning: %v", err) - } else { - err = manifestutil.CheckDir(manifest, cmd.RootDir) - if err != nil { - logf("Warning: %v", err) - } else { - // Merge the slice keys used to build the existing rootfs with the ones - // explicitly requested. - for _, s := range manifestutil.SliceKeys(manifest) { - if !slices.Contains(sliceKeys, s) { - sliceKeys = append(sliceKeys, s) - } - } - } + manifest, err := obtainManifest(release, cmd.RootDir) + if err != nil { + logf("Warning: %v", err) + } + // Merge the slice keys used to build the existing rootfs with the ones + // explicitly requested. + for _, s := range manifestutil.SliceKeys(manifest) { + if !slices.Contains(sliceKeys, s) { + sliceKeys = append(sliceKeys, s) } } - + selection, err := setup.Select(release, sliceKeys, cmd.Arch) if err != nil { return err diff --git a/cmd/chisel/helpers.go b/cmd/chisel/helpers.go index 7506888e..54783638 100644 --- a/cmd/chisel/helpers.go +++ b/cmd/chisel/helpers.go @@ -6,7 +6,9 @@ import ( "regexp" "strings" + "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/setup" + "github.com/canonical/chisel/public/manifest" ) // TODO These need testing @@ -69,3 +71,21 @@ func obtainRelease(releaseStr string) (release *setup.Release, err error) { } return release, nil } + +// obtainManifest returns the manifest extracted from the targetDir if the content +// matches the manifest. +func obtainManifest(release *setup.Release, targetDir string) (*manifest.Manifest, error) { + manifestPaths := manifestutil.FindPathsInRelease(release) + if len(manifestPaths) == 0 { + return nil, nil + } + manifest, err := manifestutil.FromDir(manifestPaths, targetDir) + if err != nil { + return nil, err + } + err = manifestutil.CheckDir(manifest, targetDir) + if err != nil { + return nil, err + } + return manifest, nil +} diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 08746f97..010b56a2 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -467,6 +467,9 @@ func contentHash(path string) ([]byte, error) { // SliceKeys returns setup.SliceKeys from a manifest. func SliceKeys(mfest *manifest.Manifest) []setup.SliceKey { + if mfest == nil { + return nil + } sliceKeys := []setup.SliceKey{} mfest.IterateSlices("", func(slice *manifest.Slice) error { sk, err := setup.ParseSliceKey(slice.Name) From 17162f4364128cb5ba10e5a4fb6b7d571c6c6b08 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 6 Jan 2026 09:15:27 +0100 Subject: [PATCH 30/54] refactor: simplify by removing SliceKeys function --- cmd/chisel/cmd_cut.go | 17 +++++++++++------ internal/manifestutil/manifestutil.go | 17 ----------------- 2 files changed, 11 insertions(+), 23 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 97c301ae..d3f47a33 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -9,9 +9,9 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/cache" - "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" + "github.com/canonical/chisel/public/manifest" ) var shortCutHelp = "Cut a tree with selected slices" @@ -74,16 +74,21 @@ func (cmd *cmdCut) Execute(args []string) error { } } - manifest, err := obtainManifest(release, cmd.RootDir) + mfest, err := obtainManifest(release, cmd.RootDir) if err != nil { logf("Warning: %v", err) } // Merge the slice keys used to build the existing rootfs with the ones // explicitly requested. - for _, s := range manifestutil.SliceKeys(manifest) { - if !slices.Contains(sliceKeys, s) { - sliceKeys = append(sliceKeys, s) - } + if mfest != nil { + mfest.IterateSlices("", func(slice *manifest.Slice) error { + sk, err := setup.ParseSliceKey(slice.Name) + if err != nil { + return err + } + sliceKeys = append(sliceKeys, sk) + return nil + }) } selection, err := setup.Select(release, sliceKeys, cmd.Arch) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 010b56a2..6e5ac09a 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -464,20 +464,3 @@ func contentHash(path string) ([]byte, error) { } return h.Sum(nil), nil } - -// SliceKeys returns setup.SliceKeys from a manifest. -func SliceKeys(mfest *manifest.Manifest) []setup.SliceKey { - if mfest == nil { - return nil - } - sliceKeys := []setup.SliceKey{} - mfest.IterateSlices("", func(slice *manifest.Slice) error { - sk, err := setup.ParseSliceKey(slice.Name) - if err != nil { - return err - } - sliceKeys = append(sliceKeys, sk) - return nil - }) - return sliceKeys -} From c21ef3468aff54710941158e488abd0a528ae6e1 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 13 Jan 2026 08:34:56 +0100 Subject: [PATCH 31/54] style: remove redondant comment --- internal/manifestutil/check.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go index bb5b4612..9a91d8cf 100644 --- a/internal/manifestutil/check.go +++ b/internal/manifestutil/check.go @@ -97,7 +97,6 @@ func CheckDir(mfest *manifest.Manifest, rootDir string) error { return err } -// recordedHash returns path.FinalSHA256 if present, otherwise path.SHA256. func recordedHash(path *manifest.Path) string { expectedHash := path.FinalSHA256 if expectedHash == "" { From 77f5771060d341cfaa58314a0368bddf31c5d6d8 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 14 Jan 2026 09:15:13 +0100 Subject: [PATCH 32/54] refactor: rework pathInfos comparison --- internal/manifestutil/check.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go index 9a91d8cf..4b294fa7 100644 --- a/internal/manifestutil/check.go +++ b/internal/manifestutil/check.go @@ -6,7 +6,6 @@ import ( "io/fs" "os" "path/filepath" - "reflect" "syscall" "github.com/canonical/chisel/public/manifest" @@ -72,8 +71,17 @@ func CheckDir(mfest *manifest.Manifest, rootDir string) error { link: link, hash: hash, } - if !reflect.DeepEqual(mfestPathInfo, fsEntryInfo) { - return fmt.Errorf("inconsistent content at %q: recorded %+v, observed %+v", path.Path, mfestPathInfo, fsEntryInfo) + if mfestPathInfo.mode != fsEntryInfo.mode { + return fmt.Errorf("inconsistent mode at %q: recorded %+v, observed %+v", path.Path, mfestPathInfo.mode, fsEntryInfo.mode) + } + if mfestPathInfo.size != fsEntryInfo.size { + return fmt.Errorf("inconsistent size at %q: recorded %+v, observed %+v", path.Path, mfestPathInfo.size, fsEntryInfo.size) + } + if mfestPathInfo.link != fsEntryInfo.link { + return fmt.Errorf("inconsistent link at %q: recorded %+v, observed %+v", path.Path, mfestPathInfo.link, fsEntryInfo.link) + } + if mfestPathInfo.hash != fsEntryInfo.hash { + return fmt.Errorf("inconsistent hash at %q: recorded %+v, observed %+v", path.Path, mfestPathInfo.hash, fsEntryInfo.hash) } // Check hardlink From 40ef8e838694a5cd1b876d1afab692a68a5ab3a5 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 14 Jan 2026 09:15:59 +0100 Subject: [PATCH 33/54] refactor: simplify FindPathsInRelease --- internal/manifestutil/manifestutil.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 6e5ac09a..055afdc4 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "io/fs" + "maps" "os" "path" "path/filepath" @@ -48,11 +49,7 @@ func FindPathsInRelease(r *setup.Release) []string { } } manifestMap := FindPaths(allSlices) - paths := make([]string, 0, len(manifestMap)) - for path := range manifestMap { - paths = append(paths, path) - } - return paths + return slices.Sorted(maps.Keys(manifestMap)) } type WriteOptions struct { From 0a258443d2ccb492976fa78c1b9378dd5f1a1612 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 14 Jan 2026 11:14:00 +0100 Subject: [PATCH 34/54] refactor: refine API --- cmd/chisel/cmd_cut.go | 35 ++++++++++++++++---------- cmd/chisel/helpers.go | 20 --------------- internal/manifestutil/manifestutil.go | 36 ++++++++++++++------------- 3 files changed, 41 insertions(+), 50 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index d3f47a33..ff576164 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -9,6 +9,7 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/cache" + "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" "github.com/canonical/chisel/public/manifest" @@ -74,21 +75,29 @@ func (cmd *cmdCut) Execute(args []string) error { } } - mfest, err := obtainManifest(release, cmd.RootDir) - if err != nil { - logf("Warning: %v", err) - } - // Merge the slice keys used to build the existing rootfs with the ones - // explicitly requested. - if mfest != nil { - mfest.IterateSlices("", func(slice *manifest.Slice) error { - sk, err := setup.ParseSliceKey(slice.Name) + manifestPaths := manifestutil.FindPathsInRelease(release) + if len(manifestPaths) >= 0 { + mfest, err := manifestutil.FromDir(manifestPaths, cmd.RootDir) + if err != nil { + return err + } + if mfest != nil { + err = manifestutil.CheckDir(mfest, cmd.RootDir) if err != nil { - return err + logf("Warning: %v", err) + } else { + // Merge the slice keys used to build the existing rootfs with the ones + // explicitly requested. + mfest.IterateSlices("", func(slice *manifest.Slice) error { + sk, err := setup.ParseSliceKey(slice.Name) + if err != nil { + return err + } + sliceKeys = append(sliceKeys, sk) + return nil + }) } - sliceKeys = append(sliceKeys, sk) - return nil - }) + } } selection, err := setup.Select(release, sliceKeys, cmd.Arch) diff --git a/cmd/chisel/helpers.go b/cmd/chisel/helpers.go index 54783638..7506888e 100644 --- a/cmd/chisel/helpers.go +++ b/cmd/chisel/helpers.go @@ -6,9 +6,7 @@ import ( "regexp" "strings" - "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/setup" - "github.com/canonical/chisel/public/manifest" ) // TODO These need testing @@ -71,21 +69,3 @@ func obtainRelease(releaseStr string) (release *setup.Release, err error) { } return release, nil } - -// obtainManifest returns the manifest extracted from the targetDir if the content -// matches the manifest. -func obtainManifest(release *setup.Release, targetDir string) (*manifest.Manifest, error) { - manifestPaths := manifestutil.FindPathsInRelease(release) - if len(manifestPaths) == 0 { - return nil, nil - } - manifest, err := manifestutil.FromDir(manifestPaths, targetDir) - if err != nil { - return nil, err - } - err = manifestutil.CheckDir(manifest, targetDir) - if err != nil { - return nil, err - } - return manifest, nil -} diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 055afdc4..fad75350 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -359,7 +359,7 @@ func Validate(mfest *manifest.Manifest) (err error) { return nil } -// FromDir extracts, checks and returns a manifest from a rootDir +// FromDir extracts, validates and returns the first manifest found in a rootDir func FromDir(manifestPaths []string, rootDir string) (*manifest.Manifest, error) { targetDir := filepath.Clean(rootDir) if !filepath.IsAbs(targetDir) { @@ -369,19 +369,27 @@ func FromDir(manifestPaths []string, rootDir string) (*manifest.Manifest, error) } targetDir = filepath.Join(dir, targetDir) } - manifestPath := manifestPaths[0] - manifest, err := load(path.Join(targetDir, manifestPath)) - if err != nil { - return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", manifestPath, err) - } - err = checkIdentical(targetDir, manifestPaths) - if err != nil { - return nil, err + var mfest *manifest.Manifest + var err error + for _, p := range manifestPaths { + manifestPath := path.Join(targetDir, p) + mfest, err = load(manifestPath) + if err != nil { + if os.IsNotExist(err) { + continue + } + return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", manifestPath, err) + } + err = Validate(mfest) + if err != nil { + return nil, err + } + return mfest, nil } - return manifest, nil + return nil, nil } -// load reads, validates and returns a manifest. +// load reads and returns a manifest. func load(manifestPath string) (*manifest.Manifest, error) { f, err := os.Open(manifestPath) if err != nil { @@ -399,12 +407,6 @@ func load(manifestPath string) (*manifest.Manifest, error) { if err != nil { return nil, err } - - err = Validate(mfest) - if err != nil { - return nil, err - } - return mfest, nil } From 71c44962733b7286eeb4491832a83e822ea7b128 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 15 Jan 2026 14:59:39 +0100 Subject: [PATCH 35/54] refactor: simplify based on PR suggestions --- internal/manifestutil/check.go | 67 ++++++++++++++++++--------- internal/manifestutil/manifestutil.go | 57 +---------------------- 2 files changed, 45 insertions(+), 79 deletions(-) diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go index 4b294fa7..ba8fd0a1 100644 --- a/internal/manifestutil/check.go +++ b/internal/manifestutil/check.go @@ -1,8 +1,10 @@ package manifestutil import ( + "crypto/sha256" "encoding/hex" "fmt" + "io" "io/fs" "os" "path/filepath" @@ -24,34 +26,43 @@ type pathInfo struct { // Files not managed by chisel are ignored. func CheckDir(mfest *manifest.Manifest, rootDir string) error { mfestInodeToFSInode := make(map[uint64]uint64) + var mfestRefHash string err := mfest.IteratePaths("", func(path *manifest.Path) error { - // Skip manifest files + fullPath := filepath.Join(rootDir, path.Path) + pathHash := recordedHash(path) if filepath.Base(path.Path) == DefaultFilename { - return nil + // Manifests must all be the same. + // Recorded hash is empty. + // So set the first computed hash as the "recorded" value. + if len(mfestRefHash) == 0 { + h, err := contentHash(fullPath) + if err != nil { + return fmt.Errorf("internal error: cannot compute hash for %q: %w", fullPath, err) + } + mfestRefHash = hex.EncodeToString(h) + } + pathHash = mfestRefHash } mfestPathInfo := &pathInfo{ mode: path.Mode, size: int64(path.Size), link: path.Link, - hash: recordedHash(path), + hash: pathHash, } - var link string - var hash string - var size int64 - var inode uint64 - fullPath := filepath.Join(rootDir, path.Path) + fsEntryInfo := &pathInfo{} info, err := os.Lstat(fullPath) if err != nil { return err } mode := info.Mode() + fsEntryInfo.mode = fmt.Sprintf("0%o", unixPerm(mode)) ftype := mode & fs.ModeType switch ftype { case fs.ModeDir: // Nothing to do case fs.ModeSymlink: - link, err = os.Readlink(fullPath) + fsEntryInfo.link, err = os.Readlink(fullPath) if err != nil { return fmt.Errorf("internal error: cannot read symlink %q: %w", fullPath, err) } @@ -60,17 +71,12 @@ func CheckDir(mfest *manifest.Manifest, rootDir string) error { if err != nil { return fmt.Errorf("internal error: cannot compute hash for %q: %w", fullPath, err) } - hash = hex.EncodeToString(h) - size = info.Size() + fsEntryInfo.hash = hex.EncodeToString(h) + fsEntryInfo.size = info.Size() default: return fmt.Errorf("inconsistent content: %q has unrecognized type %s", fullPath, mode.String()) } - fsEntryInfo := &pathInfo{ - mode: fmt.Sprintf("0%o", unixPerm(mode)), - size: size, - link: link, - hash: hash, - } + if mfestPathInfo.mode != fsEntryInfo.mode { return fmt.Errorf("inconsistent mode at %q: recorded %+v, observed %+v", path.Path, mfestPathInfo.mode, fsEntryInfo.mode) } @@ -86,13 +92,14 @@ func CheckDir(mfest *manifest.Manifest, rootDir string) error { // Check hardlink if path.Inode != 0 { - if ftype != fs.ModeDir { - stat, ok := info.Sys().(*syscall.Stat_t) - if !ok { - return fmt.Errorf("internal error: cannot get syscall stat info for %q", info.Name()) - } - inode = stat.Ino + if ftype == fs.ModeDir { + return fmt.Errorf("iconsistent type at %q: recorded a hardlinked path, observed a directory", info.Name()) } + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return fmt.Errorf("internal error: cannot get syscall stat info for %q", info.Name()) + } + inode := stat.Ino recordedInode, ok := mfestInodeToFSInode[path.Inode] if !ok { mfestInodeToFSInode[path.Inode] = inode @@ -105,6 +112,20 @@ func CheckDir(mfest *manifest.Manifest, rootDir string) error { return err } +func contentHash(path string) ([]byte, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return nil, err + } + return h.Sum(nil), nil +} + func recordedHash(path *manifest.Path) string { expectedHash := path.FinalSHA256 if expectedHash == "" { diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index fad75350..291659d0 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -1,7 +1,6 @@ package manifestutil import ( - "crypto/sha256" "fmt" "io" "io/fs" @@ -359,7 +358,7 @@ func Validate(mfest *manifest.Manifest) (err error) { return nil } -// FromDir extracts, validates and returns the first manifest found in a rootDir +// FromDir extracts, validates and returns the first manifest found in a rootDir. func FromDir(manifestPaths []string, rootDir string) (*manifest.Manifest, error) { targetDir := filepath.Clean(rootDir) if !filepath.IsAbs(targetDir) { @@ -409,57 +408,3 @@ func load(manifestPath string) (*manifest.Manifest, error) { } return mfest, nil } - -// checkIdentical checks all manifests are identical. -func checkIdentical(targetDir string, manifestPaths []string) error { - if len(manifestPaths) == 0 { - return nil - } - refRelPath := manifestPaths[0] - - ref := path.Join(targetDir, refRelPath) - refHash, err := contentHash(ref) - if err != nil { - return fmt.Errorf("internal error: cannot compute hash for %q: %w", refRelPath, err) - } - - refInfo, err := os.Stat(ref) - if err != nil { - return fmt.Errorf("internal error: cannot get file info for %q: %w", refRelPath, err) - } - refMode := refInfo.Mode() - for _, manifestRelPath := range manifestPaths { - manifestPath := path.Join(targetDir, manifestRelPath) - manifestInfo, err := os.Stat(manifestPath) - if err != nil { - return fmt.Errorf("internal error: cannot get file info for %q: %w", manifestRelPath, err) - } - manifestMode := manifestInfo.Mode() - if manifestMode != refMode { - return fmt.Errorf("invalid manifest: permissions on %s (%s) different from %s (%s)", manifestRelPath, manifestMode, refRelPath, refMode) - } - - manifestHash, err := contentHash(manifestPath) - if err != nil { - return fmt.Errorf("internal error: cannot compute hash for %q: %w", manifestRelPath, err) - } - if !slices.Equal(manifestHash, refHash) { - return fmt.Errorf("invalid manifest: %s is inconsistent with %s", manifestRelPath, refRelPath) - } - } - return nil -} - -func contentHash(path string) ([]byte, error) { - f, err := os.Open(path) - if err != nil { - return nil, err - } - defer f.Close() - - h := sha256.New() - if _, err := io.Copy(h, f); err != nil { - return nil, err - } - return h.Sum(nil), nil -} From 4696dccf0ade925883fb083973b0d48f6f989478 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 15 Jan 2026 15:59:01 +0100 Subject: [PATCH 36/54] style: tweak error messages --- internal/manifestutil/check.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go index ba8fd0a1..78546a7a 100644 --- a/internal/manifestutil/check.go +++ b/internal/manifestutil/check.go @@ -37,7 +37,7 @@ func CheckDir(mfest *manifest.Manifest, rootDir string) error { if len(mfestRefHash) == 0 { h, err := contentHash(fullPath) if err != nil { - return fmt.Errorf("internal error: cannot compute hash for %q: %w", fullPath, err) + return fmt.Errorf("cannot compute hash for %q: %w", fullPath, err) } mfestRefHash = hex.EncodeToString(h) } @@ -64,12 +64,12 @@ func CheckDir(mfest *manifest.Manifest, rootDir string) error { case fs.ModeSymlink: fsEntryInfo.link, err = os.Readlink(fullPath) if err != nil { - return fmt.Errorf("internal error: cannot read symlink %q: %w", fullPath, err) + return fmt.Errorf("cannot read symlink %q: %w", fullPath, err) } case 0: // Regular h, err := contentHash(fullPath) if err != nil { - return fmt.Errorf("internal error: cannot compute hash for %q: %w", fullPath, err) + return fmt.Errorf("cannot compute hash for %q: %w", fullPath, err) } fsEntryInfo.hash = hex.EncodeToString(h) fsEntryInfo.size = info.Size() @@ -97,7 +97,7 @@ func CheckDir(mfest *manifest.Manifest, rootDir string) error { } stat, ok := info.Sys().(*syscall.Stat_t) if !ok { - return fmt.Errorf("internal error: cannot get syscall stat info for %q", info.Name()) + return fmt.Errorf("cannot get syscall stat info for %q", info.Name()) } inode := stat.Ino recordedInode, ok := mfestInodeToFSInode[path.Inode] From 089109dfc535a7e21891a0c62ebd5d38d35d4eeb Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 16 Jan 2026 08:57:28 +0100 Subject: [PATCH 37/54] fix: fail if dir inconsistent with manifest --- cmd/chisel/cmd_cut.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index ff576164..d3586a84 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -84,7 +84,7 @@ func (cmd *cmdCut) Execute(args []string) error { if mfest != nil { err = manifestutil.CheckDir(mfest, cmd.RootDir) if err != nil { - logf("Warning: %v", err) + return err } else { // Merge the slice keys used to build the existing rootfs with the ones // explicitly requested. From 09faa6a4c637912a35450fb4808015cbee337f96 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 16 Jan 2026 14:17:22 +0100 Subject: [PATCH 38/54] fix: use loaded manifest hash as reference --- cmd/chisel/cmd_cut.go | 4 ++-- internal/manifestutil/check.go | 26 ++++++++++++-------------- internal/manifestutil/manifestutil.go | 13 +++++++------ 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index d3586a84..4075f484 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -77,12 +77,12 @@ func (cmd *cmdCut) Execute(args []string) error { manifestPaths := manifestutil.FindPathsInRelease(release) if len(manifestPaths) >= 0 { - mfest, err := manifestutil.FromDir(manifestPaths, cmd.RootDir) + mfest, mfestPath, err := manifestutil.FromDir(manifestPaths, cmd.RootDir) if err != nil { return err } if mfest != nil { - err = manifestutil.CheckDir(mfest, cmd.RootDir) + err = manifestutil.CheckDir(mfest, mfestPath, cmd.RootDir) if err != nil { return err } else { diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go index 78546a7a..a512d568 100644 --- a/internal/manifestutil/check.go +++ b/internal/manifestutil/check.go @@ -24,24 +24,22 @@ type pathInfo struct { // the manifest. // This function works under the assumption the manifest is valid. // Files not managed by chisel are ignored. -func CheckDir(mfest *manifest.Manifest, rootDir string) error { +func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error { + mfestFullPath := filepath.Join(rootDir, mfestPath) + h, err := contentHash(mfestFullPath) + if err != nil { + return fmt.Errorf("cannot compute hash for %q: %w", mfestFullPath, err) + } + mfestHash := hex.EncodeToString(h) + mfestInodeToFSInode := make(map[uint64]uint64) - var mfestRefHash string - err := mfest.IteratePaths("", func(path *manifest.Path) error { + err = mfest.IteratePaths("", func(path *manifest.Path) error { fullPath := filepath.Join(rootDir, path.Path) pathHash := recordedHash(path) if filepath.Base(path.Path) == DefaultFilename { - // Manifests must all be the same. - // Recorded hash is empty. - // So set the first computed hash as the "recorded" value. - if len(mfestRefHash) == 0 { - h, err := contentHash(fullPath) - if err != nil { - return fmt.Errorf("cannot compute hash for %q: %w", fullPath, err) - } - mfestRefHash = hex.EncodeToString(h) - } - pathHash = mfestRefHash + // Recorded hash is empty for a manifest path, + // so set the hash of the reference as the "recorded" value. + pathHash = mfestHash } mfestPathInfo := &pathInfo{ mode: path.Mode, diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 291659d0..6356d0f3 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -359,12 +359,13 @@ func Validate(mfest *manifest.Manifest) (err error) { } // FromDir extracts, validates and returns the first manifest found in a rootDir. -func FromDir(manifestPaths []string, rootDir string) (*manifest.Manifest, error) { +// Also returns the path of the manifest. +func FromDir(manifestPaths []string, rootDir string) (*manifest.Manifest, string, error) { targetDir := filepath.Clean(rootDir) if !filepath.IsAbs(targetDir) { dir, err := os.Getwd() if err != nil { - return nil, fmt.Errorf("cannot obtain current directory: %w", err) + return nil, "", fmt.Errorf("cannot obtain current directory: %w", err) } targetDir = filepath.Join(dir, targetDir) } @@ -377,15 +378,15 @@ func FromDir(manifestPaths []string, rootDir string) (*manifest.Manifest, error) if os.IsNotExist(err) { continue } - return nil, fmt.Errorf("cannot read manifest %q from the root directory: %v", manifestPath, err) + return nil, "", fmt.Errorf("cannot read manifest %q from the root directory: %v", manifestPath, err) } err = Validate(mfest) if err != nil { - return nil, err + return nil, "", err } - return mfest, nil + return mfest, p, nil } - return nil, nil + return nil, "", nil } // load reads and returns a manifest. From bd7a707f404fd4435f66cbda5a2d42d38a69f855 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 19 Jan 2026 14:00:27 +0100 Subject: [PATCH 39/54] fix: raise error when path should not be hardlinked --- internal/manifestutil/check.go | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go index a512d568..6e6bafd5 100644 --- a/internal/manifestutil/check.go +++ b/internal/manifestutil/check.go @@ -32,6 +32,7 @@ func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error } mfestHash := hex.EncodeToString(h) + singlePathsByFSInode := make(map[uint64]string) mfestInodeToFSInode := make(map[uint64]uint64) err = mfest.IteratePaths("", func(path *manifest.Path) error { fullPath := filepath.Join(rootDir, path.Path) @@ -89,20 +90,27 @@ func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error } // Check hardlink - if path.Inode != 0 { - if ftype == fs.ModeDir { - return fmt.Errorf("iconsistent type at %q: recorded a hardlinked path, observed a directory", info.Name()) - } + if ftype != fs.ModeDir { stat, ok := info.Sys().(*syscall.Stat_t) if !ok { return fmt.Errorf("cannot get syscall stat info for %q", info.Name()) } inode := stat.Ino - recordedInode, ok := mfestInodeToFSInode[path.Inode] - if !ok { - mfestInodeToFSInode[path.Inode] = inode - } else if recordedInode != inode { - return fmt.Errorf("inconsistent content at %q: file hardlinked to a different inode", path.Path) + + if path.Inode == 0 { + // This path must not be linked to any other + singlePath, ok := singlePathsByFSInode[inode] + if ok { + return fmt.Errorf("inconsistent content at %q: file hardlinked to %q", path.Path, singlePath) + } + singlePathsByFSInode[inode] = path.Path + } else { + recordedInode, ok := mfestInodeToFSInode[path.Inode] + if !ok { + mfestInodeToFSInode[path.Inode] = inode + } else if recordedInode != inode { + return fmt.Errorf("inconsistent content at %q: file hardlinked to a different inode", path.Path) + } } } return nil From 3ea70cea52a38d4fa4929bfa069cd1d8ad44eca2 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 19 Jan 2026 14:49:00 +0100 Subject: [PATCH 40/54] fix: collect and record reference manifest size --- internal/manifestutil/check.go | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go index 6e6bafd5..17e1d349 100644 --- a/internal/manifestutil/check.go +++ b/internal/manifestutil/check.go @@ -31,20 +31,27 @@ func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error return fmt.Errorf("cannot compute hash for %q: %w", mfestFullPath, err) } mfestHash := hex.EncodeToString(h) + mfestInfo, err := os.Lstat(mfestFullPath) + if err != nil { + return err + } + mfestSize := mfestInfo.Size() singlePathsByFSInode := make(map[uint64]string) mfestInodeToFSInode := make(map[uint64]uint64) err = mfest.IteratePaths("", func(path *manifest.Path) error { fullPath := filepath.Join(rootDir, path.Path) pathHash := recordedHash(path) + size := int64(path.Size) if filepath.Base(path.Path) == DefaultFilename { - // Recorded hash is empty for a manifest path, - // so set the hash of the reference as the "recorded" value. + // Recorded hash and size are empty for a manifest path, + // so set the hash and size of the reference as the "recorded" values. pathHash = mfestHash + size = mfestSize } - mfestPathInfo := &pathInfo{ + recordedPathInfo := &pathInfo{ mode: path.Mode, - size: int64(path.Size), + size: size, link: path.Link, hash: pathHash, } @@ -76,17 +83,17 @@ func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error return fmt.Errorf("inconsistent content: %q has unrecognized type %s", fullPath, mode.String()) } - if mfestPathInfo.mode != fsEntryInfo.mode { - return fmt.Errorf("inconsistent mode at %q: recorded %+v, observed %+v", path.Path, mfestPathInfo.mode, fsEntryInfo.mode) + if recordedPathInfo.mode != fsEntryInfo.mode { + return fmt.Errorf("inconsistent mode at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.mode, fsEntryInfo.mode) } - if mfestPathInfo.size != fsEntryInfo.size { - return fmt.Errorf("inconsistent size at %q: recorded %+v, observed %+v", path.Path, mfestPathInfo.size, fsEntryInfo.size) + if recordedPathInfo.size != fsEntryInfo.size { + return fmt.Errorf("inconsistent size at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.size, fsEntryInfo.size) } - if mfestPathInfo.link != fsEntryInfo.link { - return fmt.Errorf("inconsistent link at %q: recorded %+v, observed %+v", path.Path, mfestPathInfo.link, fsEntryInfo.link) + if recordedPathInfo.link != fsEntryInfo.link { + return fmt.Errorf("inconsistent link at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.link, fsEntryInfo.link) } - if mfestPathInfo.hash != fsEntryInfo.hash { - return fmt.Errorf("inconsistent hash at %q: recorded %+v, observed %+v", path.Path, mfestPathInfo.hash, fsEntryInfo.hash) + if recordedPathInfo.hash != fsEntryInfo.hash { + return fmt.Errorf("inconsistent hash at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.hash, fsEntryInfo.hash) } // Check hardlink From 3efb7d298d93b903881a33aec8a7f853cc6c631e Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 19 Jan 2026 14:49:39 +0100 Subject: [PATCH 41/54] feat: log root directory processing --- cmd/chisel/cmd_cut.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 4075f484..1674f1f5 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -77,6 +77,7 @@ func (cmd *cmdCut) Execute(args []string) error { manifestPaths := manifestutil.FindPathsInRelease(release) if len(manifestPaths) >= 0 { + logf("Processing root directory...") mfest, mfestPath, err := manifestutil.FromDir(manifestPaths, cmd.RootDir) if err != nil { return err From a421818583cc062b1a4a45be25ea497c80eeaa09 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 21 Jan 2026 12:57:45 +0100 Subject: [PATCH 42/54] refactor: more experiment around API design --- cmd/chisel/cmd_cut.go | 4 +- internal/manifestutil/check.go | 15 ++---- internal/manifestutil/manifestutil.go | 74 ++++++++++++++------------- public/manifest/manifest.go | 4 ++ 4 files changed, 50 insertions(+), 47 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 1674f1f5..a0d82c74 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -76,9 +76,9 @@ func (cmd *cmdCut) Execute(args []string) error { } manifestPaths := manifestutil.FindPathsInRelease(release) - if len(manifestPaths) >= 0 { + if len(manifestPaths) > 0 { logf("Processing root directory...") - mfest, mfestPath, err := manifestutil.FromDir(manifestPaths, cmd.RootDir) + mfest, mfestPath, err := manifestutil.FromDir(cmd.RootDir, manifestPaths) if err != nil { return err } diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go index 17e1d349..56999843 100644 --- a/internal/manifestutil/check.go +++ b/internal/manifestutil/check.go @@ -40,8 +40,10 @@ func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error singlePathsByFSInode := make(map[uint64]string) mfestInodeToFSInode := make(map[uint64]uint64) err = mfest.IteratePaths("", func(path *manifest.Path) error { - fullPath := filepath.Join(rootDir, path.Path) - pathHash := recordedHash(path) + pathHash := path.FinalSHA256 + if pathHash == "" { + pathHash = path.SHA256 + } size := int64(path.Size) if filepath.Base(path.Path) == DefaultFilename { // Recorded hash and size are empty for a manifest path, @@ -57,6 +59,7 @@ func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error } fsEntryInfo := &pathInfo{} + fullPath := filepath.Join(rootDir, path.Path) info, err := os.Lstat(fullPath) if err != nil { return err @@ -138,11 +141,3 @@ func contentHash(path string) ([]byte, error) { } return h.Sum(nil), nil } - -func recordedHash(path *manifest.Path) string { - expectedHash := path.FinalSHA256 - if expectedHash == "" { - expectedHash = path.SHA256 - } - return expectedHash -} diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 6356d0f3..65a3de87 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -358,10 +358,19 @@ func Validate(mfest *manifest.Manifest) (err error) { return nil } + +// compareSchemas compare two manifest schema strings +func compareSchemas(va, vb string) int { + if va == manifest.Schema && va == vb { + return 0 + } + return -1 +} + // FromDir extracts, validates and returns the first manifest found in a rootDir. // Also returns the path of the manifest. -func FromDir(manifestPaths []string, rootDir string) (*manifest.Manifest, string, error) { - targetDir := filepath.Clean(rootDir) +func FromDir(targetDir string, manifestPaths []string) (*manifest.Manifest, string, error) { + targetDir = filepath.Clean(targetDir) if !filepath.IsAbs(targetDir) { dir, err := os.Getwd() if err != nil { @@ -369,43 +378,38 @@ func FromDir(manifestPaths []string, rootDir string) (*manifest.Manifest, string } targetDir = filepath.Join(dir, targetDir) } - var mfest *manifest.Manifest - var err error - for _, p := range manifestPaths { - manifestPath := path.Join(targetDir, p) - mfest, err = load(manifestPath) + + var finalMfest *manifest.Manifest + var finalMfestPath string + for _, mfestPath := range manifestPaths { + var mfest *manifest.Manifest + mfestFullPath := path.Join(targetDir, mfestPath) + f, err := os.Open(mfestFullPath) if err != nil { - if os.IsNotExist(err) { - continue - } - return nil, "", fmt.Errorf("cannot read manifest %q from the root directory: %v", manifestPath, err) + // TODO: handle some errors as internal ones + // provoking a return? + continue } - err = Validate(mfest) + defer f.Close() + + r, err := zstd.NewReader(f) if err != nil { - return nil, "", err + continue } - return mfest, p, nil - } - return nil, "", nil -} + defer r.Close() -// load reads and returns a manifest. -func load(manifestPath string) (*manifest.Manifest, error) { - f, err := os.Open(manifestPath) - if err != nil { - return nil, err - } - defer f.Close() - - r, err := zstd.NewReader(f) - if err != nil { - return nil, err - } - defer r.Close() - - mfest, err := manifest.Read(r) - if err != nil { - return nil, err + mfest, err = manifest.Read(r) + if err != nil { + continue + } + err = Validate(mfest) + if err != nil { + continue + } + if finalMfest == nil || compareSchemas(mfest.Schema(), finalMfest.Schema()) > 0 { + finalMfest = mfest + finalMfestPath = mfestPath + } } - return mfest, nil + return finalMfest, finalMfestPath, nil } diff --git a/public/manifest/manifest.go b/public/manifest/manifest.go index 1e4809b8..5d832741 100644 --- a/public/manifest/manifest.go +++ b/public/manifest/manifest.go @@ -106,3 +106,7 @@ func iteratePrefix[T prefixable](manifest *Manifest, prefix *T, onMatch func(*T) } return nil } + +func (manifest *Manifest) Schema() string { + return manifest.db.Schema() +} From e88de2ddb4b3efd66ec8d15a22b0ed598e9ee678 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 21 Jan 2026 15:48:40 +0100 Subject: [PATCH 43/54] refactor: experiment to improve API --- cmd/chisel/cmd_cut.go | 32 ++-------- internal/manifestutil/check.go | 1 + internal/manifestutil/manifestutil.go | 53 +---------------- internal/slicer/slicer.go | 86 +++++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 77 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index a0d82c74..29ba08f0 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -9,10 +9,8 @@ import ( "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/cache" - "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" - "github.com/canonical/chisel/public/manifest" ) var shortCutHelp = "Cut a tree with selected slices" @@ -75,30 +73,12 @@ func (cmd *cmdCut) Execute(args []string) error { } } - manifestPaths := manifestutil.FindPathsInRelease(release) - if len(manifestPaths) > 0 { - logf("Processing root directory...") - mfest, mfestPath, err := manifestutil.FromDir(cmd.RootDir, manifestPaths) - if err != nil { - return err - } - if mfest != nil { - err = manifestutil.CheckDir(mfest, mfestPath, cmd.RootDir) - if err != nil { - return err - } else { - // Merge the slice keys used to build the existing rootfs with the ones - // explicitly requested. - mfest.IterateSlices("", func(slice *manifest.Slice) error { - sk, err := setup.ParseSliceKey(slice.Name) - if err != nil { - return err - } - sliceKeys = append(sliceKeys, sk) - return nil - }) - } - } + extractedSliceKeys, err := slicer.Extract(release, cmd.RootDir) + if err != nil { + return err + } + if extractedSliceKeys != nil { + sliceKeys = append(sliceKeys, extractedSliceKeys...) } selection, err := setup.Select(release, sliceKeys, cmd.Arch) diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go index 56999843..2851b1fa 100644 --- a/internal/manifestutil/check.go +++ b/internal/manifestutil/check.go @@ -46,6 +46,7 @@ func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error } size := int64(path.Size) if filepath.Base(path.Path) == DefaultFilename { + // TODO Also chech hash and size are empty // Recorded hash and size are empty for a manifest path, // so set the hash and size of the reference as the "recorded" values. pathHash = mfestHash diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 65a3de87..ed256958 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -5,8 +5,6 @@ import ( "io" "io/fs" "maps" - "os" - "path" "path/filepath" "slices" "sort" @@ -17,7 +15,6 @@ import ( "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/public/jsonwall" "github.com/canonical/chisel/public/manifest" - "github.com/klauspost/compress/zstd" ) const DefaultFilename = "manifest.wall" @@ -359,57 +356,11 @@ func Validate(mfest *manifest.Manifest) (err error) { } -// compareSchemas compare two manifest schema strings -func compareSchemas(va, vb string) int { +// CompareSchemas compare two manifest schema strings +func CompareSchemas(va, vb string) int { if va == manifest.Schema && va == vb { return 0 } return -1 } -// FromDir extracts, validates and returns the first manifest found in a rootDir. -// Also returns the path of the manifest. -func FromDir(targetDir string, manifestPaths []string) (*manifest.Manifest, string, error) { - targetDir = filepath.Clean(targetDir) - if !filepath.IsAbs(targetDir) { - dir, err := os.Getwd() - if err != nil { - return nil, "", fmt.Errorf("cannot obtain current directory: %w", err) - } - targetDir = filepath.Join(dir, targetDir) - } - - var finalMfest *manifest.Manifest - var finalMfestPath string - for _, mfestPath := range manifestPaths { - var mfest *manifest.Manifest - mfestFullPath := path.Join(targetDir, mfestPath) - f, err := os.Open(mfestFullPath) - if err != nil { - // TODO: handle some errors as internal ones - // provoking a return? - continue - } - defer f.Close() - - r, err := zstd.NewReader(f) - if err != nil { - continue - } - defer r.Close() - - mfest, err = manifest.Read(r) - if err != nil { - continue - } - err = Validate(mfest) - if err != nil { - continue - } - if finalMfest == nil || compareSchemas(mfest.Schema(), finalMfest.Schema()) > 0 { - finalMfest = mfest - finalMfestPath = mfestPath - } - } - return finalMfest, finalMfestPath, nil -} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 9d3447fb..faa2929e 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -7,6 +7,7 @@ import ( "io" "io/fs" "os" + "path" "path/filepath" "slices" "sort" @@ -21,6 +22,7 @@ import ( "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" "github.com/canonical/chisel/internal/setup" + "github.com/canonical/chisel/public/manifest" ) const manifestMode fs.FileMode = 0644 @@ -537,3 +539,87 @@ func selectPkgArchives(archives map[string]archive.Archive, selection *setup.Sel } return pkgArchive, nil } + +// Extract does conceptually the reverse operation as Run. +// From a target directory, return a list of SliceKeys used to build it. +func Extract(release *setup.Release, targetDir string) ([]setup.SliceKey, error) { + var sliceKeys []setup.SliceKey + manifestPaths := manifestutil.FindPathsInRelease(release) + if len(manifestPaths) > 0 { + logf("Processing root directory...") + mfest, mfestPath, err := extractValidManifest(targetDir, manifestPaths) + if err != nil { + return nil, err + } + if mfest != nil { + err = manifestutil.CheckDir(mfest, mfestPath, targetDir) + if err != nil { + return nil, err + } + // Merge the slice keys used to build the existing rootfs with the ones + // explicitly requested. + mfest.IterateSlices("", func(slice *manifest.Slice) error { + sk, err := setup.ParseSliceKey(slice.Name) + if err != nil { + return err + } + sliceKeys = append(sliceKeys, sk) + return nil + }) + + } + } + + return sliceKeys, nil +} + +// extractValidManifest extracts, validates and returns the first manifest found in a rootDir. +// Also returns the path of the manifest. +func extractValidManifest(targetDir string, manifestPaths []string) (*manifest.Manifest, string, error) { + targetDir = filepath.Clean(targetDir) + if !filepath.IsAbs(targetDir) { + dir, err := os.Getwd() + if err != nil { + return nil, "", fmt.Errorf("cannot obtain current directory: %w", err) + } + targetDir = filepath.Join(dir, targetDir) + } + + var finalMfest *manifest.Manifest + var finalMfestPath string + for _, mfestPath := range manifestPaths { + var mfest *manifest.Manifest + mfestFullPath := path.Join(targetDir, mfestPath) + f, err := os.Open(mfestFullPath) + if err != nil { + if os.IsNotExist(err) { + continue + } + return nil, "", err + } + defer f.Close() + + r, err := zstd.NewReader(f) + if err != nil { + // Fail? + continue + } + defer r.Close() + + mfest, err = manifest.Read(r) + if err != nil { + continue + } + err = manifestutil.Validate(mfest) + if err != nil { + // Fail? + continue + } + + if finalMfest == nil || manifestutil.CompareSchemas(mfest.Schema(), finalMfest.Schema()) > 0 { + finalMfest = mfest + finalMfestPath = mfestPath + } + } + return finalMfest, finalMfestPath, nil +} From acac42145616644a564af3ca5d31e342e353d59f Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 22 Jan 2026 09:42:34 +0100 Subject: [PATCH 44/54] refactor: more refining --- cmd/chisel/cmd_cut.go | 2 +- internal/manifestutil/manifestutil.go | 1 - internal/slicer/slicer.go | 17 +++++++---------- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 29ba08f0..347a781a 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -73,7 +73,7 @@ func (cmd *cmdCut) Execute(args []string) error { } } - extractedSliceKeys, err := slicer.Extract(release, cmd.RootDir) + extractedSliceKeys, err := slicer.Inspect(cmd.RootDir,release) if err != nil { return err } diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index ed256958..3272e570 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -355,7 +355,6 @@ func Validate(mfest *manifest.Manifest) (err error) { return nil } - // CompareSchemas compare two manifest schema strings func CompareSchemas(va, vb string) int { if va == manifest.Schema && va == vb { diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index faa2929e..edf8773b 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -540,13 +540,13 @@ func selectPkgArchives(archives map[string]archive.Archive, selection *setup.Sel return pkgArchive, nil } -// Extract does conceptually the reverse operation as Run. -// From a target directory, return a list of SliceKeys used to build it. -func Extract(release *setup.Release, targetDir string) ([]setup.SliceKey, error) { +// Inspect examines and validates the targetDir. +// Return the list of SliceKeys used to build the targetDir. +func Inspect(targetDir string, release *setup.Release) ([]setup.SliceKey, error) { var sliceKeys []setup.SliceKey manifestPaths := manifestutil.FindPathsInRelease(release) if len(manifestPaths) > 0 { - logf("Processing root directory...") + logf("Inspecting root directory...") mfest, mfestPath, err := extractValidManifest(targetDir, manifestPaths) if err != nil { return nil, err @@ -556,8 +556,6 @@ func Extract(release *setup.Release, targetDir string) ([]setup.SliceKey, error) if err != nil { return nil, err } - // Merge the slice keys used to build the existing rootfs with the ones - // explicitly requested. mfest.IterateSlices("", func(slice *manifest.Slice) error { sk, err := setup.ParseSliceKey(slice.Name) if err != nil { @@ -566,15 +564,14 @@ func Extract(release *setup.Release, targetDir string) ([]setup.SliceKey, error) sliceKeys = append(sliceKeys, sk) return nil }) - } } - return sliceKeys, nil } -// extractValidManifest extracts, validates and returns the first manifest found in a rootDir. -// Also returns the path of the manifest. +// extractValidManifest extracts and validates the first most recent manifest +// found in a directory. +// Returns the manigfest and its path. func extractValidManifest(targetDir string, manifestPaths []string) (*manifest.Manifest, string, error) { targetDir = filepath.Clean(targetDir) if !filepath.IsAbs(targetDir) { From 0262cb2f481b5cbd8edb635994565fe32cabc0a6 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 22 Jan 2026 14:17:39 +0100 Subject: [PATCH 45/54] refactor: CheckDir in slicer --- internal/manifestutil/check.go | 144 --------------------------------- internal/slicer/slicer.go | 6 +- 2 files changed, 2 insertions(+), 148 deletions(-) delete mode 100644 internal/manifestutil/check.go diff --git a/internal/manifestutil/check.go b/internal/manifestutil/check.go deleted file mode 100644 index 2851b1fa..00000000 --- a/internal/manifestutil/check.go +++ /dev/null @@ -1,144 +0,0 @@ -package manifestutil - -import ( - "crypto/sha256" - "encoding/hex" - "fmt" - "io" - "io/fs" - "os" - "path/filepath" - "syscall" - - "github.com/canonical/chisel/public/manifest" -) - -type pathInfo struct { - mode string - size int64 - link string - hash string -} - -// CheckDir checks the content of the target directory matches with -// the manifest. -// This function works under the assumption the manifest is valid. -// Files not managed by chisel are ignored. -func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error { - mfestFullPath := filepath.Join(rootDir, mfestPath) - h, err := contentHash(mfestFullPath) - if err != nil { - return fmt.Errorf("cannot compute hash for %q: %w", mfestFullPath, err) - } - mfestHash := hex.EncodeToString(h) - mfestInfo, err := os.Lstat(mfestFullPath) - if err != nil { - return err - } - mfestSize := mfestInfo.Size() - - singlePathsByFSInode := make(map[uint64]string) - mfestInodeToFSInode := make(map[uint64]uint64) - err = mfest.IteratePaths("", func(path *manifest.Path) error { - pathHash := path.FinalSHA256 - if pathHash == "" { - pathHash = path.SHA256 - } - size := int64(path.Size) - if filepath.Base(path.Path) == DefaultFilename { - // TODO Also chech hash and size are empty - // Recorded hash and size are empty for a manifest path, - // so set the hash and size of the reference as the "recorded" values. - pathHash = mfestHash - size = mfestSize - } - recordedPathInfo := &pathInfo{ - mode: path.Mode, - size: size, - link: path.Link, - hash: pathHash, - } - - fsEntryInfo := &pathInfo{} - fullPath := filepath.Join(rootDir, path.Path) - info, err := os.Lstat(fullPath) - if err != nil { - return err - } - mode := info.Mode() - fsEntryInfo.mode = fmt.Sprintf("0%o", unixPerm(mode)) - ftype := mode & fs.ModeType - switch ftype { - case fs.ModeDir: - // Nothing to do - case fs.ModeSymlink: - fsEntryInfo.link, err = os.Readlink(fullPath) - if err != nil { - return fmt.Errorf("cannot read symlink %q: %w", fullPath, err) - } - case 0: // Regular - h, err := contentHash(fullPath) - if err != nil { - return fmt.Errorf("cannot compute hash for %q: %w", fullPath, err) - } - fsEntryInfo.hash = hex.EncodeToString(h) - fsEntryInfo.size = info.Size() - default: - return fmt.Errorf("inconsistent content: %q has unrecognized type %s", fullPath, mode.String()) - } - - if recordedPathInfo.mode != fsEntryInfo.mode { - return fmt.Errorf("inconsistent mode at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.mode, fsEntryInfo.mode) - } - if recordedPathInfo.size != fsEntryInfo.size { - return fmt.Errorf("inconsistent size at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.size, fsEntryInfo.size) - } - if recordedPathInfo.link != fsEntryInfo.link { - return fmt.Errorf("inconsistent link at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.link, fsEntryInfo.link) - } - if recordedPathInfo.hash != fsEntryInfo.hash { - return fmt.Errorf("inconsistent hash at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.hash, fsEntryInfo.hash) - } - - // Check hardlink - if ftype != fs.ModeDir { - stat, ok := info.Sys().(*syscall.Stat_t) - if !ok { - return fmt.Errorf("cannot get syscall stat info for %q", info.Name()) - } - inode := stat.Ino - - if path.Inode == 0 { - // This path must not be linked to any other - singlePath, ok := singlePathsByFSInode[inode] - if ok { - return fmt.Errorf("inconsistent content at %q: file hardlinked to %q", path.Path, singlePath) - } - singlePathsByFSInode[inode] = path.Path - } else { - recordedInode, ok := mfestInodeToFSInode[path.Inode] - if !ok { - mfestInodeToFSInode[path.Inode] = inode - } else if recordedInode != inode { - return fmt.Errorf("inconsistent content at %q: file hardlinked to a different inode", path.Path) - } - } - } - return nil - }) - return err -} - -func contentHash(path string) ([]byte, error) { - f, err := os.Open(path) - if err != nil { - return nil, err - } - defer f.Close() - - h := sha256.New() - if _, err := io.Copy(h, f); err != nil { - return nil, err - } - return h.Sum(nil), nil -} diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index edf8773b..813704e2 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -552,7 +552,7 @@ func Inspect(targetDir string, release *setup.Release) ([]setup.SliceKey, error) return nil, err } if mfest != nil { - err = manifestutil.CheckDir(mfest, mfestPath, targetDir) + err = CheckDir(mfest, mfestPath, targetDir) if err != nil { return nil, err } @@ -598,8 +598,7 @@ func extractValidManifest(targetDir string, manifestPaths []string) (*manifest.M r, err := zstd.NewReader(f) if err != nil { - // Fail? - continue + return nil, "", err } defer r.Close() @@ -609,7 +608,6 @@ func extractValidManifest(targetDir string, manifestPaths []string) (*manifest.M } err = manifestutil.Validate(mfest) if err != nil { - // Fail? continue } From 7445fcdb0985b1c67ff2358568645a068aa48ac5 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 22 Jan 2026 14:18:37 +0100 Subject: [PATCH 46/54] refactor: add check in slicer --- internal/slicer/check.go | 194 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 194 insertions(+) create mode 100644 internal/slicer/check.go diff --git a/internal/slicer/check.go b/internal/slicer/check.go new file mode 100644 index 00000000..d0e51b53 --- /dev/null +++ b/internal/slicer/check.go @@ -0,0 +1,194 @@ +package slicer + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "syscall" + + "github.com/klauspost/compress/zstd" + + "github.com/canonical/chisel/internal/manifestutil" + "github.com/canonical/chisel/public/manifest" +) + +type pathInfo struct { + mode string + size int64 + link string + hash string +} + +type manifestInfo struct { + path string + size int64 + hash string +} + +func unixPerm(mode fs.FileMode) (perm uint32) { + perm = uint32(mode.Perm()) + if mode&fs.ModeSticky != 0 { + perm |= 0o1000 + } + return perm +} + +// CheckDir checks the content of the target directory matches with +// the manifest. +// This function works under the assumption the manifest is valid. +// Files not managed by chisel are ignored. +func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error { + singlePathsByFSInode := make(map[uint64]string) + mfestInodeToFSInode := make(map[uint64]uint64) + manifestInfos := make(map[string]*pathInfo) + err := mfest.IteratePaths("", func(path *manifest.Path) error { + pathHash := path.FinalSHA256 + if pathHash == "" { + pathHash = path.SHA256 + } + size := int64(path.Size) + recordedPathInfo := &pathInfo{ + mode: path.Mode, + size: size, + link: path.Link, + hash: pathHash, + } + + fsEntryInfo := &pathInfo{} + fullPath := filepath.Join(rootDir, path.Path) + info, err := os.Lstat(fullPath) + if err != nil { + return err + } + mode := info.Mode() + fsEntryInfo.mode = fmt.Sprintf("0%o", unixPerm(mode)) + ftype := mode & fs.ModeType + switch ftype { + case fs.ModeDir: + // Nothing to do + case fs.ModeSymlink: + fsEntryInfo.link, err = os.Readlink(fullPath) + if err != nil { + return fmt.Errorf("cannot read symlink %q: %w", fullPath, err) + } + case 0: // Regular + h, err := contentHash(fullPath) + if err != nil { + return fmt.Errorf("cannot compute hash for %q: %w", fullPath, err) + } + fsEntryInfo.hash = hex.EncodeToString(h) + fsEntryInfo.size = info.Size() + default: + return fmt.Errorf("inconsistent content: %q has unrecognized type %s", fullPath, mode.String()) + } + + // Collect manifests for tailored checking later. + // Adjust observed hash and size to still compare in a generic way. + if filepath.Base(path.Path) == manifestutil.DefaultFilename && size == 0 && pathHash == "" { + manifestInfos[path.Path] = &(*fsEntryInfo) + fsEntryInfo.size = 0 + fsEntryInfo.hash = "" + } + + if recordedPathInfo.mode != fsEntryInfo.mode { + return fmt.Errorf("inconsistent mode at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.mode, fsEntryInfo.mode) + } + if recordedPathInfo.size != fsEntryInfo.size { + return fmt.Errorf("inconsistent size at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.size, fsEntryInfo.size) + } + if recordedPathInfo.link != fsEntryInfo.link { + return fmt.Errorf("inconsistent link at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.link, fsEntryInfo.link) + } + if recordedPathInfo.hash != fsEntryInfo.hash { + return fmt.Errorf("inconsistent hash at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.hash, fsEntryInfo.hash) + } + + // Check hardlink + if ftype != fs.ModeDir { + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return fmt.Errorf("cannot get syscall stat info for %q", info.Name()) + } + inode := stat.Ino + + if path.Inode == 0 { + // This path must not be linked to any other + singlePath, ok := singlePathsByFSInode[inode] + if ok { + return fmt.Errorf("inconsistent content at %q: file hardlinked to %q", path.Path, singlePath) + } + singlePathsByFSInode[inode] = path.Path + } else { + recordedInode, ok := mfestInodeToFSInode[path.Inode] + if !ok { + mfestInodeToFSInode[path.Inode] = inode + } else if recordedInode != inode { + return fmt.Errorf("inconsistent content at %q: file hardlinked to a different inode", path.Path) + } + } + } + + return nil + }) + + // Check manifests + // Manifests are compared per schema version. + schemasToManifestInfo := make(map[string]manifestInfo) + + // They must all be existing, readable and valid manifests. + // They must be consistent per schema version. + for path, info := range manifestInfos { + fullPath := filepath.Join(rootDir, path) + f, err := os.Open(fullPath) + if err != nil { + return err + } + defer f.Close() + r, err := zstd.NewReader(f) + if err != nil { + return err + } + defer r.Close() + mfest, err = manifest.Read(r) + if err != nil { + return err + } + err = manifestutil.Validate(mfest) + if err != nil { + return err + } + schema := mfest.Schema() + refInfo, ok := schemasToManifestInfo[schema] + if !ok { + schemasToManifestInfo[schema] = refInfo + continue + } + + if refInfo.size != info.size { + return fmt.Errorf("inconsistent manifest size for version %s at %q: recorded %+v, observed %+v", schema, path, refInfo.size, info.size) + } + if refInfo.hash != info.hash { + return fmt.Errorf("inconsistent manifest hash for version %s at %q: recorded %+v, observed %+v", schema, path, refInfo.hash, info.hash) + } + + } + return err +} + +func contentHash(path string) ([]byte, error) { + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return nil, err + } + return h.Sum(nil), nil +} From 5fca0c5bc17267c9c13f386e6650dc50b921724a Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 22 Jan 2026 14:54:58 +0100 Subject: [PATCH 47/54] refactor: simplify and reduce slicer API --- internal/slicer/check.go | 42 +++++++++++++++++---------------------- internal/slicer/slicer.go | 18 ++++++++--------- 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/internal/slicer/check.go b/internal/slicer/check.go index d0e51b53..4e27338b 100644 --- a/internal/slicer/check.go +++ b/internal/slicer/check.go @@ -23,12 +23,6 @@ type pathInfo struct { hash string } -type manifestInfo struct { - path string - size int64 - hash string -} - func unixPerm(mode fs.FileMode) (perm uint32) { perm = uint32(mode.Perm()) if mode&fs.ModeSticky != 0 { @@ -37,11 +31,11 @@ func unixPerm(mode fs.FileMode) (perm uint32) { return perm } -// CheckDir checks the content of the target directory matches with +// checkDir checks the content of the target directory matches with // the manifest. // This function works under the assumption the manifest is valid. // Files not managed by chisel are ignored. -func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error { +func checkDir(mfest *manifest.Manifest, rootDir string) error { singlePathsByFSInode := make(map[uint64]string) mfestInodeToFSInode := make(map[uint64]uint64) manifestInfos := make(map[string]*pathInfo) @@ -50,10 +44,9 @@ func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error if pathHash == "" { pathHash = path.SHA256 } - size := int64(path.Size) recordedPathInfo := &pathInfo{ mode: path.Mode, - size: size, + size: int64(path.Size), link: path.Link, hash: pathHash, } @@ -88,23 +81,25 @@ func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error // Collect manifests for tailored checking later. // Adjust observed hash and size to still compare in a generic way. - if filepath.Base(path.Path) == manifestutil.DefaultFilename && size == 0 && pathHash == "" { - manifestInfos[path.Path] = &(*fsEntryInfo) + if filepath.Base(path.Path) == manifestutil.DefaultFilename && recordedPathInfo.size == 0 && recordedPathInfo.hash == "" { + var mfestInfo *pathInfo + *mfestInfo = *fsEntryInfo + manifestInfos[path.Path] = mfestInfo fsEntryInfo.size = 0 fsEntryInfo.hash = "" } if recordedPathInfo.mode != fsEntryInfo.mode { - return fmt.Errorf("inconsistent mode at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.mode, fsEntryInfo.mode) + return fmt.Errorf("inconsistent mode at %q: recorded %v, observed %v", path.Path, recordedPathInfo.mode, fsEntryInfo.mode) } if recordedPathInfo.size != fsEntryInfo.size { - return fmt.Errorf("inconsistent size at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.size, fsEntryInfo.size) + return fmt.Errorf("inconsistent size at %q: recorded %v, observed %v", path.Path, recordedPathInfo.size, fsEntryInfo.size) } if recordedPathInfo.link != fsEntryInfo.link { - return fmt.Errorf("inconsistent link at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.link, fsEntryInfo.link) + return fmt.Errorf("inconsistent link at %q: recorded %v, observed %v", path.Path, recordedPathInfo.link, fsEntryInfo.link) } if recordedPathInfo.hash != fsEntryInfo.hash { - return fmt.Errorf("inconsistent hash at %q: recorded %+v, observed %+v", path.Path, recordedPathInfo.hash, fsEntryInfo.hash) + return fmt.Errorf("inconsistent hash at %q: recorded %v, observed %v", path.Path, recordedPathInfo.hash, fsEntryInfo.hash) } // Check hardlink @@ -131,16 +126,16 @@ func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error } } } - return nil }) + if err != nil { + return err + } // Check manifests - // Manifests are compared per schema version. - schemasToManifestInfo := make(map[string]manifestInfo) - // They must all be existing, readable and valid manifests. // They must be consistent per schema version. + schemasToManifestInfo := make(map[string]pathInfo) for path, info := range manifestInfos { fullPath := filepath.Join(rootDir, path) f, err := os.Open(fullPath) @@ -169,14 +164,13 @@ func CheckDir(mfest *manifest.Manifest, mfestPath string, rootDir string) error } if refInfo.size != info.size { - return fmt.Errorf("inconsistent manifest size for version %s at %q: recorded %+v, observed %+v", schema, path, refInfo.size, info.size) + return fmt.Errorf("inconsistent manifest size for version %s at %q: recorded %v, observed %v", schema, path, refInfo.size, info.size) } if refInfo.hash != info.hash { - return fmt.Errorf("inconsistent manifest hash for version %s at %q: recorded %+v, observed %+v", schema, path, refInfo.hash, info.hash) + return fmt.Errorf("inconsistent manifest hash for version %s at %q: recorded %v, observed %v", schema, path, refInfo.hash, info.hash) } - } - return err + return nil } func contentHash(path string) ([]byte, error) { diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 813704e2..a8d1df73 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -547,12 +547,12 @@ func Inspect(targetDir string, release *setup.Release) ([]setup.SliceKey, error) manifestPaths := manifestutil.FindPathsInRelease(release) if len(manifestPaths) > 0 { logf("Inspecting root directory...") - mfest, mfestPath, err := extractValidManifest(targetDir, manifestPaths) + mfest, err := extractValidManifest(targetDir, manifestPaths) if err != nil { return nil, err } if mfest != nil { - err = CheckDir(mfest, mfestPath, targetDir) + err = checkDir(mfest, targetDir) if err != nil { return nil, err } @@ -571,19 +571,18 @@ func Inspect(targetDir string, release *setup.Release) ([]setup.SliceKey, error) // extractValidManifest extracts and validates the first most recent manifest // found in a directory. -// Returns the manigfest and its path. -func extractValidManifest(targetDir string, manifestPaths []string) (*manifest.Manifest, string, error) { +// If found, returns the manifest. +func extractValidManifest(targetDir string, manifestPaths []string) (*manifest.Manifest, error) { targetDir = filepath.Clean(targetDir) if !filepath.IsAbs(targetDir) { dir, err := os.Getwd() if err != nil { - return nil, "", fmt.Errorf("cannot obtain current directory: %w", err) + return nil, fmt.Errorf("cannot obtain current directory: %w", err) } targetDir = filepath.Join(dir, targetDir) } var finalMfest *manifest.Manifest - var finalMfestPath string for _, mfestPath := range manifestPaths { var mfest *manifest.Manifest mfestFullPath := path.Join(targetDir, mfestPath) @@ -592,13 +591,13 @@ func extractValidManifest(targetDir string, manifestPaths []string) (*manifest.M if os.IsNotExist(err) { continue } - return nil, "", err + return nil, err } defer f.Close() r, err := zstd.NewReader(f) if err != nil { - return nil, "", err + return nil, err } defer r.Close() @@ -613,8 +612,7 @@ func extractValidManifest(targetDir string, manifestPaths []string) (*manifest.M if finalMfest == nil || manifestutil.CompareSchemas(mfest.Schema(), finalMfest.Schema()) > 0 { finalMfest = mfest - finalMfestPath = mfestPath } } - return finalMfest, finalMfestPath, nil + return finalMfest, nil } From fb6500b7072d5de03f82d08df8cd0032674d4f66 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Thu, 22 Jan 2026 16:11:24 +0100 Subject: [PATCH 48/54] ci: rerun From d7fa15575f6c5ec39860236c119d51104abf3c62 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 23 Jan 2026 08:50:23 +0100 Subject: [PATCH 49/54] wip --- internal/slicer/slicer.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index a8d1df73..3366c12a 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -569,9 +569,8 @@ func Inspect(targetDir string, release *setup.Release) ([]setup.SliceKey, error) return sliceKeys, nil } -// extractValidManifest extracts and validates the first most recent manifest -// found in a directory. -// If found, returns the manifest. +// extractValidManifest extracts and validates manifests found in a directory. +// If found, returns the first manifest with the latest schema version. func extractValidManifest(targetDir string, manifestPaths []string) (*manifest.Manifest, error) { targetDir = filepath.Clean(targetDir) if !filepath.IsAbs(targetDir) { @@ -613,6 +612,9 @@ func extractValidManifest(targetDir string, manifestPaths []string) (*manifest.M if finalMfest == nil || manifestutil.CompareSchemas(mfest.Schema(), finalMfest.Schema()) > 0 { finalMfest = mfest } + // TODO: keep all valid ones, compare them + // make sure they list each other + // TODO: sort them and pick the first one to be deterministic } return finalMfest, nil } From 43e8a71beee5e14174dca9a6dcb973f82fff1913 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 23 Jan 2026 13:16:02 +0100 Subject: [PATCH 50/54] refactor: cleaning --- internal/slicer/check.go | 8 ++--- internal/slicer/slicer.go | 66 +++++++++++++++++++-------------------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/internal/slicer/check.go b/internal/slicer/check.go index 4e27338b..7a884889 100644 --- a/internal/slicer/check.go +++ b/internal/slicer/check.go @@ -133,9 +133,9 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { } // Check manifests - // They must all be existing, readable and valid manifests. + // They must all be valid manifests. // They must be consistent per schema version. - schemasToManifestInfo := make(map[string]pathInfo) + schemaManifestInfos := make(map[string]pathInfo) for path, info := range manifestInfos { fullPath := filepath.Join(rootDir, path) f, err := os.Open(fullPath) @@ -157,9 +157,9 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { return err } schema := mfest.Schema() - refInfo, ok := schemasToManifestInfo[schema] + refInfo, ok := schemaManifestInfos[schema] if !ok { - schemasToManifestInfo[schema] = refInfo + schemaManifestInfos[schema] = refInfo continue } diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 3366c12a..8566d029 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -547,7 +547,7 @@ func Inspect(targetDir string, release *setup.Release) ([]setup.SliceKey, error) manifestPaths := manifestutil.FindPathsInRelease(release) if len(manifestPaths) > 0 { logf("Inspecting root directory...") - mfest, err := extractValidManifest(targetDir, manifestPaths) + mfest, err := selectValidManifest(targetDir, manifestPaths) if err != nil { return nil, err } @@ -569,9 +569,9 @@ func Inspect(targetDir string, release *setup.Release) ([]setup.SliceKey, error) return sliceKeys, nil } -// extractValidManifest extracts and validates manifests found in a directory. -// If found, returns the first manifest with the latest schema version. -func extractValidManifest(targetDir string, manifestPaths []string) (*manifest.Manifest, error) { +// selectValidManifest the first valid manifest found in a directory with the +// latest schema version. +func selectValidManifest(targetDir string, manifestPaths []string) (*manifest.Manifest, error) { targetDir = filepath.Clean(targetDir) if !filepath.IsAbs(targetDir) { dir, err := os.Getwd() @@ -581,40 +581,40 @@ func extractValidManifest(targetDir string, manifestPaths []string) (*manifest.M targetDir = filepath.Join(dir, targetDir) } - var finalMfest *manifest.Manifest + var selected *manifest.Manifest for _, mfestPath := range manifestPaths { - var mfest *manifest.Manifest - mfestFullPath := path.Join(targetDir, mfestPath) - f, err := os.Open(mfestFullPath) - if err != nil { - if os.IsNotExist(err) { - continue + err := func() error { + mfestFullPath := path.Join(targetDir, mfestPath) + f, err := os.Open(mfestFullPath) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return err + } + defer f.Close() + r, err := zstd.NewReader(f) + if err != nil { + return err + } + defer r.Close() + mfest, err := manifest.Read(r) + if err != nil { + return nil + } + err = manifestutil.Validate(mfest) + if err != nil { + return nil } - return nil, err - } - defer f.Close() - r, err := zstd.NewReader(f) + if selected == nil || manifestutil.CompareSchemas(mfest.Schema(), selected.Schema()) > 0 { + selected = mfest + } + return nil + }() if err != nil { return nil, err } - defer r.Close() - - mfest, err = manifest.Read(r) - if err != nil { - continue - } - err = manifestutil.Validate(mfest) - if err != nil { - continue - } - - if finalMfest == nil || manifestutil.CompareSchemas(mfest.Schema(), finalMfest.Schema()) > 0 { - finalMfest = mfest - } - // TODO: keep all valid ones, compare them - // make sure they list each other - // TODO: sort them and pick the first one to be deterministic } - return finalMfest, nil + return selected, nil } From 46d21195e63961ffecc83722bce2250cf95df4c1 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 23 Jan 2026 16:58:16 +0100 Subject: [PATCH 51/54] fix: check manifest consistency at selection --- internal/slicer/check.go | 46 +++++++++++++++++++-------------------- internal/slicer/slicer.go | 17 +++++++++++++++ 2 files changed, 39 insertions(+), 24 deletions(-) diff --git a/internal/slicer/check.go b/internal/slicer/check.go index 7a884889..b874e811 100644 --- a/internal/slicer/check.go +++ b/internal/slicer/check.go @@ -37,7 +37,7 @@ func unixPerm(mode fs.FileMode) (perm uint32) { // Files not managed by chisel are ignored. func checkDir(mfest *manifest.Manifest, rootDir string) error { singlePathsByFSInode := make(map[uint64]string) - mfestInodeToFSInode := make(map[uint64]uint64) + fsInodeByManifestInode := make(map[uint64]uint64) manifestInfos := make(map[string]*pathInfo) err := mfest.IteratePaths("", func(path *manifest.Path) error { pathHash := path.FinalSHA256 @@ -51,20 +51,20 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { hash: pathHash, } - fsEntryInfo := &pathInfo{} + fsInfo := &pathInfo{} fullPath := filepath.Join(rootDir, path.Path) info, err := os.Lstat(fullPath) if err != nil { return err } mode := info.Mode() - fsEntryInfo.mode = fmt.Sprintf("0%o", unixPerm(mode)) + fsInfo.mode = fmt.Sprintf("0%o", unixPerm(mode)) ftype := mode & fs.ModeType switch ftype { case fs.ModeDir: // Nothing to do case fs.ModeSymlink: - fsEntryInfo.link, err = os.Readlink(fullPath) + fsInfo.link, err = os.Readlink(fullPath) if err != nil { return fmt.Errorf("cannot read symlink %q: %w", fullPath, err) } @@ -73,8 +73,8 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { if err != nil { return fmt.Errorf("cannot compute hash for %q: %w", fullPath, err) } - fsEntryInfo.hash = hex.EncodeToString(h) - fsEntryInfo.size = info.Size() + fsInfo.hash = hex.EncodeToString(h) + fsInfo.size = info.Size() default: return fmt.Errorf("inconsistent content: %q has unrecognized type %s", fullPath, mode.String()) } @@ -82,26 +82,24 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { // Collect manifests for tailored checking later. // Adjust observed hash and size to still compare in a generic way. if filepath.Base(path.Path) == manifestutil.DefaultFilename && recordedPathInfo.size == 0 && recordedPathInfo.hash == "" { - var mfestInfo *pathInfo - *mfestInfo = *fsEntryInfo - manifestInfos[path.Path] = mfestInfo - fsEntryInfo.size = 0 - fsEntryInfo.hash = "" + mfestInfo := *fsInfo + manifestInfos[path.Path] = &mfestInfo + fsInfo.size = 0 + fsInfo.hash = "" } - if recordedPathInfo.mode != fsEntryInfo.mode { - return fmt.Errorf("inconsistent mode at %q: recorded %v, observed %v", path.Path, recordedPathInfo.mode, fsEntryInfo.mode) + if recordedPathInfo.mode != fsInfo.mode { + return fmt.Errorf("inconsistent mode at %q: recorded %v, observed %v", path.Path, recordedPathInfo.mode, fsInfo.mode) } - if recordedPathInfo.size != fsEntryInfo.size { - return fmt.Errorf("inconsistent size at %q: recorded %v, observed %v", path.Path, recordedPathInfo.size, fsEntryInfo.size) + if recordedPathInfo.size != fsInfo.size { + return fmt.Errorf("inconsistent size at %q: recorded %v, observed %v", path.Path, recordedPathInfo.size, fsInfo.size) } - if recordedPathInfo.link != fsEntryInfo.link { - return fmt.Errorf("inconsistent link at %q: recorded %v, observed %v", path.Path, recordedPathInfo.link, fsEntryInfo.link) + if recordedPathInfo.link != fsInfo.link { + return fmt.Errorf("inconsistent link at %q: recorded %v, observed %v", path.Path, recordedPathInfo.link, fsInfo.link) } - if recordedPathInfo.hash != fsEntryInfo.hash { - return fmt.Errorf("inconsistent hash at %q: recorded %v, observed %v", path.Path, recordedPathInfo.hash, fsEntryInfo.hash) + if recordedPathInfo.hash != fsInfo.hash { + return fmt.Errorf("inconsistent hash at %q: recorded %v, observed %v", path.Path, recordedPathInfo.hash, fsInfo.hash) } - // Check hardlink if ftype != fs.ModeDir { stat, ok := info.Sys().(*syscall.Stat_t) @@ -118,9 +116,9 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { } singlePathsByFSInode[inode] = path.Path } else { - recordedInode, ok := mfestInodeToFSInode[path.Inode] + recordedInode, ok := fsInodeByManifestInode[path.Inode] if !ok { - mfestInodeToFSInode[path.Inode] = inode + fsInodeByManifestInode[path.Inode] = inode } else if recordedInode != inode { return fmt.Errorf("inconsistent content at %q: file hardlinked to a different inode", path.Path) } @@ -135,7 +133,7 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { // Check manifests // They must all be valid manifests. // They must be consistent per schema version. - schemaManifestInfos := make(map[string]pathInfo) + schemaManifestInfos := make(map[string]*pathInfo) for path, info := range manifestInfos { fullPath := filepath.Join(rootDir, path) f, err := os.Open(fullPath) @@ -159,7 +157,7 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { schema := mfest.Schema() refInfo, ok := schemaManifestInfos[schema] if !ok { - schemaManifestInfos[schema] = refInfo + schemaManifestInfos[schema] = info continue } diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 8566d029..b9f06687 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -3,6 +3,7 @@ package slicer import ( "archive/tar" "bytes" + "encoding/hex" "fmt" "io" "io/fs" @@ -581,7 +582,12 @@ func selectValidManifest(targetDir string, manifestPaths []string) (*manifest.Ma targetDir = filepath.Join(dir, targetDir) } + type manifestHash struct { + path string + hash string + } var selected *manifest.Manifest + schemaManifest := make(map[string]manifestHash) for _, mfestPath := range manifestPaths { err := func() error { mfestFullPath := path.Join(targetDir, mfestPath) @@ -608,6 +614,17 @@ func selectValidManifest(targetDir string, manifestPaths []string) (*manifest.Ma } if selected == nil || manifestutil.CompareSchemas(mfest.Schema(), selected.Schema()) > 0 { + h, err := contentHash(mfestFullPath) + if err != nil { + return fmt.Errorf("cannot compute hash for %q: %w", mfestFullPath, err) + } + mfestHash := hex.EncodeToString(h) + refMfest, ok := schemaManifest[mfest.Schema()] + if !ok { + schemaManifest[mfest.Schema()] = manifestHash{mfestPath, mfestHash} + } else if refMfest.hash != mfestHash { + return fmt.Errorf("inconsistent manifests: %q and %q", refMfest.path, mfestPath) + } selected = mfest } return nil From 164c68b5fa5527240ec5790dc0a6eae81a1d9f40 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 26 Jan 2026 09:35:24 +0100 Subject: [PATCH 52/54] refactor: return a manifest from inspection --- cmd/chisel/cmd_cut.go | 14 +++++++++++--- internal/slicer/slicer.go | 19 +++++++------------ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 347a781a..cca9055f 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -11,6 +11,7 @@ import ( "github.com/canonical/chisel/internal/cache" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" + "github.com/canonical/chisel/public/manifest" ) var shortCutHelp = "Cut a tree with selected slices" @@ -73,12 +74,19 @@ func (cmd *cmdCut) Execute(args []string) error { } } - extractedSliceKeys, err := slicer.Inspect(cmd.RootDir,release) + mfest, err := slicer.Inspect(cmd.RootDir, release) if err != nil { return err } - if extractedSliceKeys != nil { - sliceKeys = append(sliceKeys, extractedSliceKeys...) + if mfest != nil { + mfest.IterateSlices("", func(slice *manifest.Slice) error { + sk, err := setup.ParseSliceKey(slice.Name) + if err != nil { + return err + } + sliceKeys = append(sliceKeys, sk) + return nil + }) } selection, err := setup.Select(release, sliceKeys, cmd.Arch) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index b9f06687..143c77ae 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -543,12 +543,13 @@ func selectPkgArchives(archives map[string]archive.Archive, selection *setup.Sel // Inspect examines and validates the targetDir. // Return the list of SliceKeys used to build the targetDir. -func Inspect(targetDir string, release *setup.Release) ([]setup.SliceKey, error) { - var sliceKeys []setup.SliceKey +func Inspect(targetDir string, release *setup.Release) (*manifest.Manifest, error) { + var mfest *manifest.Manifest manifestPaths := manifestutil.FindPathsInRelease(release) if len(manifestPaths) > 0 { logf("Inspecting root directory...") - mfest, err := selectValidManifest(targetDir, manifestPaths) + var err error + mfest, err = selectValidManifest(targetDir, manifestPaths) if err != nil { return nil, err } @@ -557,21 +558,15 @@ func Inspect(targetDir string, release *setup.Release) ([]setup.SliceKey, error) if err != nil { return nil, err } - mfest.IterateSlices("", func(slice *manifest.Slice) error { - sk, err := setup.ParseSliceKey(slice.Name) - if err != nil { - return err - } - sliceKeys = append(sliceKeys, sk) - return nil - }) } } - return sliceKeys, nil + return mfest, nil } // selectValidManifest the first valid manifest found in a directory with the // latest schema version. +// A manifest is considered valid if it is consistent with others present and +// of the same schema. func selectValidManifest(targetDir string, manifestPaths []string) (*manifest.Manifest, error) { targetDir = filepath.Clean(targetDir) if !filepath.IsAbs(targetDir) { From 15550adf6ec01ec4c9032df221bed54838f707e3 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 26 Jan 2026 13:23:39 +0100 Subject: [PATCH 53/54] refactor: apply PR suggestions --- internal/manifestutil/manifestutil.go | 2 +- internal/slicer/check.go | 28 +++++++++++++-------------- internal/slicer/slicer.go | 13 ++++++------- public/manifest/manifest.go | 8 ++++---- 4 files changed, 24 insertions(+), 27 deletions(-) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index 3272e570..e8777c7b 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -355,7 +355,7 @@ func Validate(mfest *manifest.Manifest) (err error) { return nil } -// CompareSchemas compare two manifest schema strings +// CompareSchemas compares two manifest schema strings. func CompareSchemas(va, vb string) int { if va == manifest.Schema && va == vb { return 0 diff --git a/internal/slicer/check.go b/internal/slicer/check.go index b874e811..ccf97605 100644 --- a/internal/slicer/check.go +++ b/internal/slicer/check.go @@ -31,11 +31,10 @@ func unixPerm(mode fs.FileMode) (perm uint32) { return perm } -// checkDir checks the content of the target directory matches with -// the manifest. +// checkRootDir checks the content of the target directory matches with +// the manifest. Files not managed by chisel are ignored. // This function works under the assumption the manifest is valid. -// Files not managed by chisel are ignored. -func checkDir(mfest *manifest.Manifest, rootDir string) error { +func checkRootDir(mfest *manifest.Manifest, rootDir string) error { singlePathsByFSInode := make(map[uint64]string) fsInodeByManifestInode := make(map[uint64]uint64) manifestInfos := make(map[string]*pathInfo) @@ -62,13 +61,13 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { ftype := mode & fs.ModeType switch ftype { case fs.ModeDir: - // Nothing to do + // Nothing to do. case fs.ModeSymlink: fsInfo.link, err = os.Readlink(fullPath) if err != nil { return fmt.Errorf("cannot read symlink %q: %w", fullPath, err) } - case 0: // Regular + case 0: // Regular file. h, err := contentHash(fullPath) if err != nil { return fmt.Errorf("cannot compute hash for %q: %w", fullPath, err) @@ -76,11 +75,11 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { fsInfo.hash = hex.EncodeToString(h) fsInfo.size = info.Size() default: - return fmt.Errorf("inconsistent content: %q has unrecognized type %s", fullPath, mode.String()) + return fmt.Errorf("cannot check %q: unrecognized type %s", fullPath, mode.String()) } - // Collect manifests for tailored checking later. - // Adjust observed hash and size to still compare in a generic way. + // Collect manifests for tailored checking later. Adjust observed hash and + // size to still compare in a generic way. if filepath.Base(path.Path) == manifestutil.DefaultFilename && recordedPathInfo.size == 0 && recordedPathInfo.hash == "" { mfestInfo := *fsInfo manifestInfos[path.Path] = &mfestInfo @@ -100,7 +99,7 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { if recordedPathInfo.hash != fsInfo.hash { return fmt.Errorf("inconsistent hash at %q: recorded %v, observed %v", path.Path, recordedPathInfo.hash, fsInfo.hash) } - // Check hardlink + // Check hardlink. if ftype != fs.ModeDir { stat, ok := info.Sys().(*syscall.Stat_t) if !ok { @@ -109,10 +108,10 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { inode := stat.Ino if path.Inode == 0 { - // This path must not be linked to any other + // This path must not be linked to any other. singlePath, ok := singlePathsByFSInode[inode] if ok { - return fmt.Errorf("inconsistent content at %q: file hardlinked to %q", path.Path, singlePath) + return fmt.Errorf("inconsistent content at %q: recorded no hardlink, observed hardlinked to %q", path.Path, singlePath) } singlePathsByFSInode[inode] = path.Path } else { @@ -130,9 +129,8 @@ func checkDir(mfest *manifest.Manifest, rootDir string) error { return err } - // Check manifests - // They must all be valid manifests. - // They must be consistent per schema version. + // Check manifests. + // They must all be valid manifests and be consistent per schema version. schemaManifestInfos := make(map[string]*pathInfo) for path, info := range manifestInfos { fullPath := filepath.Join(rootDir, path) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 143c77ae..48eb0e11 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -541,8 +541,8 @@ func selectPkgArchives(archives map[string]archive.Archive, selection *setup.Sel return pkgArchive, nil } -// Inspect examines and validates the targetDir. -// Return the list of SliceKeys used to build the targetDir. +// Inspect examines and validates the targetDir. It returns, if found and valid +// the manifest representing the content in the targetDir. func Inspect(targetDir string, release *setup.Release) (*manifest.Manifest, error) { var mfest *manifest.Manifest manifestPaths := manifestutil.FindPathsInRelease(release) @@ -554,7 +554,7 @@ func Inspect(targetDir string, release *setup.Release) (*manifest.Manifest, erro return nil, err } if mfest != nil { - err = checkDir(mfest, targetDir) + err = checkRootDir(mfest, targetDir) if err != nil { return nil, err } @@ -563,10 +563,9 @@ func Inspect(targetDir string, release *setup.Release) (*manifest.Manifest, erro return mfest, nil } -// selectValidManifest the first valid manifest found in a directory with the -// latest schema version. -// A manifest is considered valid if it is consistent with others present and -// of the same schema. +// selectValidManifest returns, if found, a valid manifest with the latest +// schema. Consistency with all other manifests with the same schema is verified +// so the selection is deterministic. func selectValidManifest(targetDir string, manifestPaths []string) (*manifest.Manifest, error) { targetDir = filepath.Clean(targetDir) if !filepath.IsAbs(targetDir) { diff --git a/public/manifest/manifest.go b/public/manifest/manifest.go index 5d832741..65b362d5 100644 --- a/public/manifest/manifest.go +++ b/public/manifest/manifest.go @@ -68,6 +68,10 @@ func Read(reader io.Reader) (manifest *Manifest, err error) { return manifest, nil } +func (manifest *Manifest) Schema() string { + return manifest.db.Schema() +} + func (manifest *Manifest) IteratePaths(pathPrefix string, onMatch func(*Path) error) (err error) { return iteratePrefix(manifest, &Path{Kind: "path", Path: pathPrefix}, onMatch) } @@ -106,7 +110,3 @@ func iteratePrefix[T prefixable](manifest *Manifest, prefix *T, onMatch func(*T) } return nil } - -func (manifest *Manifest) Schema() string { - return manifest.db.Schema() -} From 4ecb5557e5296a8146e59a3fd0da36f371ad38a0 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 26 Jan 2026 14:48:41 +0100 Subject: [PATCH 54/54] refactor: avoid iterating twice on slices --- internal/manifestutil/manifestutil.go | 32 +++++++++++++++++---------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/internal/manifestutil/manifestutil.go b/internal/manifestutil/manifestutil.go index e8777c7b..f6bb7e60 100644 --- a/internal/manifestutil/manifestutil.go +++ b/internal/manifestutil/manifestutil.go @@ -4,7 +4,6 @@ import ( "fmt" "io" "io/fs" - "maps" "path/filepath" "slices" "sort" @@ -19,18 +18,25 @@ import ( const DefaultFilename = "manifest.wall" +func collectManifests(slice *setup.Slice, collector func(path string, slice *setup.Slice)) { + for path, info := range slice.Contents { + if info.Generate == setup.GenerateManifest { + dir := strings.TrimSuffix(path, "**") + path = filepath.Join(dir, DefaultFilename) + collector(path, slice) + } + } +} + // FindPaths finds the paths marked with "generate:manifest" and // returns a map from the manifest path to all the slices that declare it. func FindPaths(slices []*setup.Slice) map[string][]*setup.Slice { manifestSlices := make(map[string][]*setup.Slice) + collector := func(path string, slice *setup.Slice) { + manifestSlices[path] = append(manifestSlices[path], slice) + } for _, slice := range slices { - for path, info := range slice.Contents { - if info.Generate == setup.GenerateManifest { - dir := strings.TrimSuffix(path, "**") - path = filepath.Join(dir, DefaultFilename) - manifestSlices[path] = append(manifestSlices[path], slice) - } - } + collectManifests(slice, collector) } return manifestSlices } @@ -38,14 +44,16 @@ func FindPaths(slices []*setup.Slice) map[string][]*setup.Slice { // FindPathsInRelease finds all the paths marked with "generate:manifest" // for the given release. func FindPathsInRelease(r *setup.Release) []string { - allSlices := []*setup.Slice{} + manifestPaths := make([]string,0) + collector := func(path string, slice *setup.Slice) { + manifestPaths = append(manifestPaths, path) + } for _, pkg := range r.Packages { for _, slice := range pkg.Slices { - allSlices = append(allSlices, slice) + collectManifests(slice, collector) } } - manifestMap := FindPaths(allSlices) - return slices.Sorted(maps.Keys(manifestMap)) + return manifestPaths } type WriteOptions struct {