diff --git a/engine/resources/file.go b/engine/resources/file.go index e7120989..a4b2657a 100644 --- a/engine/resources/file.go +++ b/engine/resources/file.go @@ -54,6 +54,12 @@ const ( // FileStateUndefined means the file state has not been specified. // TODO: consider moving to *string and express this state as a nil. FileStateUndefined = "" + + // FileModeAllowAssign specifies whether we only use ugo=rwx style + // assignment (false) or if we also allow ugo+-rwx style too (true). I + // think that it's possibly illogical to allow imperative mode + // specifiers in a declarative language, so let's leave it off for now. + FileModeAllowAssign = false ) // FileRes is a file and directory resource. Dirs are defined by names ending @@ -173,15 +179,14 @@ func (obj *FileRes) mode() (os.FileMode, error) { return os.FileMode(n), nil } - // Try parsing symbolic by first getting the files current mode. + // Try parsing symbolically by first getting the files current mode. stat, err := os.Stat(obj.getPath()) if err != nil { return os.FileMode(0), errwrap.Wrapf(err, "failed to get the current file mode") } - m := stat.Mode() modes := strings.Split(obj.Mode, ",") - m, err = engineUtil.ParseSymbolicModes(modes, m, false) + m, err := engineUtil.ParseSymbolicModes(modes, stat.Mode(), FileModeAllowAssign) if err != nil { return os.FileMode(0), errwrap.Wrapf(err, "mode should be an octal number or symbolic mode (%s)", obj.Mode) } diff --git a/engine/util/mode.go b/engine/util/mode.go index 76051701..e731707f 100644 --- a/engine/util/mode.go +++ b/engine/util/mode.go @@ -43,16 +43,16 @@ const ( // false if not. func modeIsValidWho(who string) bool { for _, w := range []string{"u", "g", "o"} { - who = strings.Replace(who, w, "", -1) + who = strings.Replace(who, w, "", -1) // TODO: use ReplaceAll in 1.12 } return len(who) == 0 } // modeIsValidWhat checks that only the valid mode characters are in the what -// string ('w', 'r', 'x', 's', 't'). +// string ('r', 'w', 'x', 's', 't'). func modeIsValidWhat(what string) bool { - for _, w := range []string{"w", "r", "x", "s", "t"} { - what = strings.Replace(what, w, "", -1) + for _, w := range []string{"r", "w", "x", "s", "t"} { + what = strings.Replace(what, w, "", -1) // TODO: use ReplaceAll in 1.12 } return len(what) == 0 } @@ -77,13 +77,13 @@ func modeAssigned(who, what string, from os.FileMode) (os.FileMode, error) { for _, c := range what { switch c { - case 'w': - m := modeValueFrom(who, ModeWrite) + case 'r': + m := modeValueFrom(who, ModeRead) if from&m == 0 { from = from | m } - case 'r': - m := modeValueFrom(who, ModeRead) + case 'w': + m := modeValueFrom(who, ModeWrite) if from&m == 0 { from = from | m } @@ -116,13 +116,13 @@ func modeAssigned(who, what string, from os.FileMode) (os.FileMode, error) { func modeAdded(who, what string, from os.FileMode) (os.FileMode, error) { for _, c := range what { switch c { - case 'w': - m := modeValueFrom(who, ModeWrite) + case 'r': + m := modeValueFrom(who, ModeRead) if from&m == 0 { from = from | m } - case 'r': - m := modeValueFrom(who, ModeRead) + case 'w': + m := modeValueFrom(who, ModeWrite) if from&m == 0 { from = from | m } @@ -155,13 +155,13 @@ func modeAdded(who, what string, from os.FileMode) (os.FileMode, error) { func modeSubtracted(who, what string, from os.FileMode) (os.FileMode, error) { for _, c := range what { switch c { - case 'w': - m := modeValueFrom(who, ModeWrite) + case 'r': + m := modeValueFrom(who, ModeRead) if from&m != 0 { from = from &^ m } - case 'r': - m := modeValueFrom(who, ModeRead) + case 'w': + m := modeValueFrom(who, ModeWrite) if from&m != 0 { from = from &^ m } @@ -214,18 +214,18 @@ func modeValueFrom(who string, modeType uint32) os.FileMode { } // ParseSymbolicModes parses a slice of symbolic mode strings. By default it -// will accept all symbolic mode strings (=,+,-) but can be set to only accept -// the assignment input with the 'onlyAssign' bool. +// will only accept the assignment input (=), but if allowAssign is true, then +// all symbolic mode strings (=,+,-) can be used as well. // // Symbolic mode is expected to be a string of who (user, group, other) then // the operation (=, +, -) then the change (read, write, execute, setuid, // setgid, sticky). // -// Ex. ug=rw +// Eg: ug=rw // // If you repeat yourself in the slice (ex. u=rw,u=w) ParseSymbolicModes will // fail with an error. -func ParseSymbolicModes(modes []string, from os.FileMode, onlyAssign bool) (os.FileMode, error) { +func ParseSymbolicModes(modes []string, from os.FileMode, allowAssign bool) (os.FileMode, error) { symModes := make([]struct { mode, who, what string @@ -235,7 +235,8 @@ func ParseSymbolicModes(modes []string, from os.FileMode, onlyAssign bool) (os.F for i, mode := range modes { symModes[i].mode = mode - // If string contains '=' and no '+/-' it is safe to guess it is an assign + // If string contains '=' and no '+/-' it is safe to guess it is + // an assign. if strings.Contains(mode, "=") && !strings.ContainsAny(mode, "+-") { m := strings.Split(mode, "=") if len(m) != 2 { @@ -245,7 +246,8 @@ func ParseSymbolicModes(modes []string, from os.FileMode, onlyAssign bool) (os.F symModes[i].what = m[1] symModes[i].parse = modeAssigned continue - } else if strings.Contains(mode, "+") && !strings.ContainsAny(mode, "=-") && !onlyAssign { + + } else if strings.Contains(mode, "+") && !strings.ContainsAny(mode, "=-") && allowAssign { m := strings.Split(mode, "+") if len(m) != 2 { return os.FileMode(0), fmt.Errorf("only a single %q is allowed but found %d", "+", len(m)) @@ -254,7 +256,8 @@ func ParseSymbolicModes(modes []string, from os.FileMode, onlyAssign bool) (os.F symModes[i].what = m[1] symModes[i].parse = modeAdded continue - } else if strings.Contains(mode, "-") && !strings.ContainsAny(mode, "=+") && !onlyAssign { + + } else if strings.Contains(mode, "-") && !strings.ContainsAny(mode, "=+") && allowAssign { m := strings.Split(mode, "-") if len(m) != 2 { return os.FileMode(0), fmt.Errorf("only a single %q is allowed but found %d", "-", len(m)) @@ -268,13 +271,14 @@ func ParseSymbolicModes(modes []string, from os.FileMode, onlyAssign bool) (os.F return os.FileMode(0), fmt.Errorf("%s is not a valid a symbolic mode", symModes[i].mode) } - // Validate input and verify the slice of symbolic modes does not contain - // redundancy. - seen := make(map[rune]struct{}) // To validate all subjects are not dupicated + // Validate input and verify the slice of symbolic modes does not + // contain redundancy. + seen := make(map[rune]struct{}) // validate that subjects are not duplicated for i := range symModes { if strings.ContainsRune(symModes[i].who, 'a') || symModes[i].who == "" { - // If 'a' or empty who (implicit 'a') is called and there are more - // symbolic modes in the slice then it must be a repetition. + // If 'a' or empty who (implicit 'a') is called and + // there are more symbolic modes in the slice then it + // must be a repetition. if len(symModes) > 1 { return os.FileMode(0), fmt.Errorf("subject was repeated: each subject (u,g,o) is only accepted once") } @@ -294,7 +298,7 @@ func ParseSymbolicModes(modes []string, from os.FileMode, onlyAssign bool) (os.F } } - // Parse each sybolic mode accumulatively onto the from file mode. + // Parse each symbolic mode accumulatively onto the file mode. for _, m := range symModes { var err error from, err = m.parse(m.who, m.what, from) diff --git a/engine/util/mode_test.go b/engine/util/mode_test.go index 62c4997d..fd3f0279 100644 --- a/engine/util/mode_test.go +++ b/engine/util/mode_test.go @@ -15,25 +15,25 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -package util_test +// +build !root + +package util import ( "fmt" "os" "strings" "testing" - - engineutil "github.com/purpleidea/mgmt/engine/util" ) func TestSymbolicMode(t *testing.T) { def := os.FileMode(0644) | os.ModeSetgid symModeTests := []struct { - name string - input []string - expect os.FileMode - onlyAssign bool - err error + name string + input []string + expect os.FileMode + allowAssign bool + err error }{ // Test single mode inputs. {"assign", []string{"a=rwx"}, 0777, false, nil}, @@ -42,36 +42,36 @@ func TestSymbolicMode(t *testing.T) { {"assign", []string{"ug=trwx"}, 0774 | os.ModeSticky, false, nil}, {"assign", []string{"o=rx"}, 0645 | os.ModeSetgid, false, nil}, {"assign", []string{"ug=srwx"}, 0774 | os.ModeSetgid | os.ModeSetuid, false, nil}, - {"addition", []string{"o+rwx"}, 0647 | os.ModeSetgid, false, nil}, - {"addition", []string{"u+x"}, 0744 | os.ModeSetgid, false, nil}, - {"addition", []string{"u+x"}, 0744 | os.ModeSetgid, false, nil}, - {"addition", []string{"u+s"}, 0644 | os.ModeSetgid | os.ModeSetuid, false, nil}, - {"addition", []string{"u+t"}, 0644 | os.ModeSetgid | os.ModeSticky, false, nil}, - {"subtraction", []string{"o-rwx"}, 0640 | os.ModeSetgid, false, nil}, - {"subtraction", []string{"u-w"}, 0444 | os.ModeSetgid, false, nil}, - {"subtraction", []string{"g-s"}, 0644, false, nil}, - {"subtraction", []string{"u-t"}, 0644 | os.ModeSetgid, false, nil}, + {"addition", []string{"o+rwx"}, 0647 | os.ModeSetgid, true, nil}, + {"addition", []string{"u+x"}, 0744 | os.ModeSetgid, true, nil}, + {"addition", []string{"u+x"}, 0744 | os.ModeSetgid, true, nil}, + {"addition", []string{"u+s"}, 0644 | os.ModeSetgid | os.ModeSetuid, true, nil}, + {"addition", []string{"u+t"}, 0644 | os.ModeSetgid | os.ModeSticky, true, nil}, + {"subtraction", []string{"o-rwx"}, 0640 | os.ModeSetgid, true, nil}, + {"subtraction", []string{"u-w"}, 0444 | os.ModeSetgid, true, nil}, + {"subtraction", []string{"g-s"}, 0644, true, nil}, + {"subtraction", []string{"u-t"}, 0644 | os.ModeSetgid, true, nil}, // Test multiple mode inputs. - {"mixed", []string{"u=rwx", "g+w"}, 0764 | os.ModeSetgid, false, nil}, - {"mixed", []string{"u+rwx", "g=w"}, 0724, false, nil}, + {"mixed", []string{"u=rwx", "g+w"}, 0764 | os.ModeSetgid, true, nil}, + {"mixed", []string{"u+rwx", "g=w"}, 0724, true, nil}, - // Test that a engineutil.ModeError is returned. Value is not checked so the - // empty string works. - {"invalid separator", []string{"ug_rwx"}, os.FileMode(0), false, fmt.Errorf("ug_rwx is not a valid a symbolic mode")}, - {"invalid who", []string{"xg=rwx"}, os.FileMode(0), false, fmt.Errorf("unexpected character assignment in xg=rwx")}, - {"invalid what", []string{"g=rwy"}, os.FileMode(0), false, fmt.Errorf("unexpected character assignment in g=rwy")}, - {"double assignment", []string{"a=rwx", "u=r"}, os.FileMode(0), false, fmt.Errorf("subject was repeated: each subject (u,g,o) is only accepted once")}, + // Test that a ModeError is returned. Value is not checked so + // the empty string works. + {"invalid separator", []string{"ug_rwx"}, os.FileMode(0), true, fmt.Errorf("ug_rwx is not a valid a symbolic mode")}, + {"invalid who", []string{"xg=rwx"}, os.FileMode(0), true, fmt.Errorf("unexpected character assignment in xg=rwx")}, + {"invalid what", []string{"g=rwy"}, os.FileMode(0), true, fmt.Errorf("unexpected character assignment in g=rwy")}, + {"double assignment", []string{"a=rwx", "u=r"}, os.FileMode(0), true, fmt.Errorf("subject was repeated: each subject (u,g,o) is only accepted once")}, - // Test onlyAssign bool - {"only assign", []string{"u+x", "g=rw"}, os.FileMode(0), true, fmt.Errorf("u+x is not a valid a symbolic mode")}, - {"not only assign", []string{"u+x", "g=rw"}, os.FileMode(0764), false, nil}, + // Test allowAssign bool. + {"only assign", []string{"u+x", "g=rw"}, os.FileMode(0), false, fmt.Errorf("u+x is not a valid a symbolic mode")}, + {"not only assign", []string{"u+x", "g=rw"}, os.FileMode(0764), true, nil}, } for _, ts := range symModeTests { test := ts t.Run(test.name+" "+strings.Join(test.input, ","), func(t *testing.T) { - got, err := engineutil.ParseSymbolicModes(test.input, def, test.onlyAssign) + got, err := ParseSymbolicModes(test.input, def, test.allowAssign) if test.err != nil { if err == nil { t.Errorf("input: %s, expected error: %#v, but got nil", def, test.err)