From b06e4de5973aa5572a4c87bb007fec13d3635c68 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Tue, 16 Sep 2025 23:57:31 +1000 Subject: [PATCH] cmd: validate: add --strict mode Due to FreeBSD compatibility, we have so far been forced into some unfortunate behaviour with regards to which comparison errors we actually return an error code for. FreeBSD generally does not return errors if a file or keyword only exists in one of the manifests being compared. Some users do not care about FreeBSD compatibility and just want to get errors if there is any difference between a manifest and directory tree. Commit 9cdd9152b3d9 ("cmd: gomtree: re-enable errors when there is a Modified entry") added a TODO to implement this, but I've only just got around to implementing it. This patch adds a new --strict flag to "gomtree validate", which causes it to error out if any changes were detected and also disables the FreeBSD compatibility keyword delta filters. This is more akin to what modern users would expect from a manifest validation tool. Fixes: 21723a3974bc ("*: fix comparison of missing keywords") Fixes: 9cdd9152b3d9 ("cmd: gomtree: re-enable errors when there is a Modified entry") Signed-off-by: Aleksa Sarai --- cmd/gomtree/cmd/validate.go | 43 +++++++------- test/cli/0013.sh | 114 ++++++++++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 21 deletions(-) create mode 100644 test/cli/0013.sh diff --git a/cmd/gomtree/cmd/validate.go b/cmd/gomtree/cmd/validate.go index bbff281..70cdcff 100644 --- a/cmd/gomtree/cmd/validate.go +++ b/cmd/gomtree/cmd/validate.go @@ -3,6 +3,7 @@ package cmd import ( "bytes" "encoding/json" + "errors" "fmt" "io" "os" @@ -81,10 +82,16 @@ func NewValidateCommand() *cli.Command { Value: "bsd", Usage: "output the validation results using the given format (bsd, json, path)", }, + &cli.BoolFlag{ + Name: "strict", + Usage: "enable strict validation of manifests (any discrepancy will result in an error)", + }, }, } } +var errValidate = errors.New("manifest validation failed") + func validateAction(c *cli.Context) error { // -list-keywords if c.Bool("list-keywords") { @@ -307,18 +314,14 @@ func validateAction(c *cli.Context) error { if err != nil { return err } - if res != nil { - out := formatFunc(res) + if len(res) > 0 { + out := formatFunc(res, c.Bool("strict")) if _, err := os.Stdout.Write([]byte(out)); err != nil { return err } - - // TODO: This should be a flag. Allowing files to be added and - // removed and still returning "it's all good" is simply - // unsafe IMO. for _, diff := range res { - if diff.Type() == mtree.Modified { - return fmt.Errorf("manifest validation failed") + if c.Bool("strict") || diff.Type() == mtree.Modified { + return errValidate } } } @@ -368,21 +371,19 @@ func validateAction(c *cli.Context) error { if isTarSpec(specDh) || c.String("tar") != "" { filters = append(filters, tarKeywordFilter) } - filters = append(filters, freebsdCompatKeywordFilter) + if !c.Bool("strict") { + filters = append(filters, freebsdCompatKeywordFilter) + } res = filterDeltas(res, filters...) if len(res) > 0 { - out := formatFunc(res) + out := formatFunc(res, c.Bool("strict")) if _, err := os.Stdout.Write([]byte(out)); err != nil { return err } - - // TODO: This should be a flag. Allowing files to be added and - // removed and still returning "it's all good" is simply - // unsafe IMO. for _, diff := range res { - if diff.Type() == mtree.Modified { - return fmt.Errorf("manifest validation failed") + if c.Bool("strict") || diff.Type() == mtree.Modified { + return errValidate } } } @@ -392,9 +393,9 @@ func validateAction(c *cli.Context) error { return nil } -var formats = map[string]func([]mtree.InodeDelta) string{ +var formats = map[string]func([]mtree.InodeDelta, bool) string{ // Outputs the errors in the BSD format. - "bsd": func(d []mtree.InodeDelta) string { + "bsd": func(d []mtree.InodeDelta, strict bool) string { var buffer bytes.Buffer for _, delta := range d { fmt.Fprintln(&buffer, delta) @@ -403,7 +404,7 @@ var formats = map[string]func([]mtree.InodeDelta) string{ }, // Outputs the full result struct in JSON. - "json": func(d []mtree.InodeDelta) string { + "json": func(d []mtree.InodeDelta, strict bool) string { var buffer bytes.Buffer if err := json.NewEncoder(&buffer).Encode(d); err != nil { panic(err) @@ -412,10 +413,10 @@ var formats = map[string]func([]mtree.InodeDelta) string{ }, // Outputs only the paths which failed to validate. - "path": func(d []mtree.InodeDelta) string { + "path": func(d []mtree.InodeDelta, strict bool) string { var buffer bytes.Buffer for _, delta := range d { - if delta.Type() == mtree.Modified { + if strict || delta.Type() == mtree.Modified { fmt.Fprintln(&buffer, delta.Path()) } } diff --git a/test/cli/0013.sh b/test/cli/0013.sh new file mode 100644 index 0000000..6b77555 --- /dev/null +++ b/test/cli/0013.sh @@ -0,0 +1,114 @@ +#!/bin/bash +set -ex + +name=$(basename $0) +root="$(dirname $(dirname $(dirname $0)))" +gomtree=$(go run ${root}/test/realpath/main.go ${root}/gomtree) +t=$(mktemp -d /tmp/go-mtree.XXXXXX) + +setfattr -n user.has.xattrs -v "true" "${t}" || exit 0 + +echo "[${name}] Running in ${t}" + +## Test that --strict mode will error out in cases that stock gomtree will not. + +pushd ${root} +mkdir -p ${t}/root + +mkdir -p ${t}/root/foo/bar +echo "valid" >${t}/root/foo/bar/file +echo "#!/bin/false" >${t}/root/binary +chmod 755 ${t}/root/binary +date="2025-09-05T13:05:10" +touch -d "$date.12345" ${t}/root/time +touch ${t}/root/xattr +setfattr -n user.mtree.testing -v "apples and=bananas" ${t}/root/xattr + +keywords_notime=type,uid,gid,nlink,link,mode,flags,xattr,size,sha256 +keywords=$keywords_notime,time +${gomtree} -c -k "$keywords" -p ${t}/root -f ${t}/root.mtree + +# Make sure cp -ar still validates. +dir=${t}/copy; cp -ar ${t}/root ${dir} +${gomtree} -k "$keywords" -p ${dir} -f ${t}/root.mtree +${gomtree} --strict -k "$keywords" -p ${dir} -f ${t}/root.mtree + +# Missing files should not validate. +dir=${t}/missing; cp -ar ${t}/root ${dir} +rm ${dir}/binary +(! ${gomtree} --strict -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} --strict -k "$keywords" -p ${dir} -f ${t}/root.mtree) +# "size" will tip off gomtree even in stock mode, so re-do with just "type". +${gomtree} -k type -p ${dir} -f ${t}/root.mtree +(! ${gomtree} --strict -k type -p ${dir} -f ${t}/root.mtree) + +# Extra files should not validate. +dir=${t}/extra; cp -ar ${t}/root ${dir} +touch ${dir}/newfile +(! ${gomtree} --strict -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} --strict -k "$keywords" -p ${dir} -f ${t}/root.mtree) +# "size" will tip off gomtree even in stock mode, so re-do with just "type". +${gomtree} -k type -p ${dir} -f ${t}/root.mtree +(! ${gomtree} --strict -k type -p ${dir} -f ${t}/root.mtree) + +# Extra keywords that are missing from the original manifest should not +# validate. +dir=${t}/root +${gomtree} -k "$keywords,md5" -p ${dir} -f ${t}/root.mtree +(! ${gomtree} --strict -k "$keywords,md5" -p ${dir} -f ${t}/root.mtree) + +# Mismatched keywords should not validate when comparing two manifests. +dir=${t}/root +keywords2=type,uid,gid,nlink,mode,flags,xattr,size,sha384,time +${gomtree} -c -k "$keywords2" -p ${dir} -f ${t}/root.mtree2 +${gomtree} -k "$keywords" -f ${t}/root.mtree -f ${t}/root.mtree2 +(! ${gomtree} --strict -k "$keywords" -f ${t}/root.mtree -f ${t}/root.mtree2) +${gomtree} -k "$keywords" -f ${t}/root.mtree2 -f ${t}/root.mtree +(! ${gomtree} --strict -k "$keywords" -f ${t}/root.mtree2 -f ${t}/root.mtree) +${gomtree} -k "$keywords2" -f ${t}/root.mtree -f ${t}/root.mtree2 +(! ${gomtree} --strict -k "$keywords2" -f ${t}/root.mtree -f ${t}/root.mtree2) +${gomtree} -k "$keywords2" -f ${t}/root.mtree2 -f ${t}/root.mtree +(! ${gomtree} --strict -k "$keywords2" -f ${t}/root.mtree2 -f ${t}/root.mtree) + +# Changed xattrs should not validate (even without --strict). +dir=${t}/xattr-change; cp -ar ${t}/root ${dir} +setfattr -n user.mtree.testing -v "different value" ${dir}/xattr +(! ${gomtree} -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} -k "$keywords" -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} --strict -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} --strict -k "$keywords" -p ${dir} -f ${t}/root.mtree) + +# Adding xattrs to existing xattr-set files should not validate (even without +# --strict). +dir=${t}/xattr-add1; cp -ar ${t}/root ${dir} +setfattr -n user.mtree.new -v "newxattr" ${dir}/xattr +(! ${gomtree} -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} -k "$keywords" -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} --strict -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} --strict -k "$keywords" -p ${dir} -f ${t}/root.mtree) + +# Adding xattrs to unrelated files should not validate (even without --strict). +dir=${t}/xattr-add2; cp -ar ${t}/root ${dir} +setfattr -n user.mtree.new -v "newxattr" ${dir}/binary +(! ${gomtree} -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} -k "$keywords" -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} --strict -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} --strict -k "$keywords" -p ${dir} -f ${t}/root.mtree) + +# Removing xattrs should not validate (even without --strict). +dir=${t}/xattr-rm; cp -ar ${t}/root ${dir} +setfattr -x user.mtree.testing ${dir}/xattr +(! ${gomtree} -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} -k "$keywords" -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} --strict -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} --strict -k "$keywords" -p ${dir} -f ${t}/root.mtree) + +# time -> tar_time validation should still work even with --strict mode. +dir=${t}/tartime; cp -ar ${t}/root ${dir} +touch -d "$date.00000" ${dir}/time +(! ${gomtree} -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} -k "$keywords" -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} --strict -p ${dir} -f ${t}/root.mtree) +(! ${gomtree} --strict -k "$keywords" -p ${dir} -f ${t}/root.mtree) +${gomtree} -k "$keywords_notime,tar_time" -p ${dir} -f ${t}/root.mtree +${gomtree} --strict -k "$keywords_notime,tar_time" -p ${dir} -f ${t}/root.mtree