This is an automated email from the ASF dual-hosted git repository. sjanc pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mynewt-newt.git
commit f48ae34b9dcd8a10b6bb0b1b907d0373cd592291 Author: Szymon Janc <[email protected]> AuthorDate: Mon Oct 19 17:46:52 2026 +0200 newt/cli: Fix target create/copy overwriting files ResolveNewTargetName, used by "target create" and "target copy", only checked whether a target with the given name already existed. Creating a target named e.g. "apps/app" silently replaced the pkg.yml of an existing non-target package, and names like "", "/", "x/.." or "../../x" wrote pkg.yml and target.yml into the targets directory, the project root or outside of the project. Reject empty names and names that are not clean relative paths or contain ".." elements, and refuse to use a destination directory that already exists and is not empty. Assisted-by: Claude:claude-opus-5-5 --- newt/cli/util.go | 39 ++++++++++++++ newt/target_name_test.go | 133 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+) diff --git a/newt/cli/util.go b/newt/cli/util.go index ccbb0bc4..452f3c97 100644 --- a/newt/cli/util.go +++ b/newt/cli/util.go @@ -24,6 +24,7 @@ import ( "bytes" "fmt" "os" + "path" "regexp" "strings" @@ -151,6 +152,23 @@ func ResolveNewTargetName(name string) (string, error) { "must be local") } + if pkgName == "" { + return "", util.NewNewtError("Target name cannot be empty") + } + + // Reject names that are not clean relative paths; e.g., ".", "x/..", + // "../x" or "/x" would place the target outside of a dedicated + // directory inside the project. + if path.IsAbs(pkgName) || path.Clean(pkgName) != pkgName || + strings.Contains(pkgName, "\\") { + return "", util.FmtNewtError("Invalid target name: %s", name) + } + for _, elem := range strings.Split(pkgName, "/") { + if elem == "." || elem == ".." { + return "", util.FmtNewtError("Invalid target name: %s", name) + } + } + if pkgName == TARGET_KEYWORD_ALL { return "", util.NewNewtError("Target name " + TARGET_KEYWORD_ALL + " is reserved") @@ -165,6 +183,27 @@ func ResolveNewTargetName(name string) (string, error) { return "", util.NewNewtError("Target already exists: " + pkgName) } + // The destination directory must not contain anything yet; otherwise + // saving the target would overwrite an existing package or files. + dir := TryGetProject().LocalRepo().Path() + "/" + pkgName + if info, err := os.Stat(dir); err == nil { + if !info.IsDir() { + return "", util.FmtNewtError( + "Target destination exists and is not a directory: %s", + pkgName) + } + entries, err := os.ReadDir(dir) + if err != nil { + return "", util.ChildNewtError(err) + } + if len(entries) > 0 { + return "", util.FmtNewtError( + "Target destination directory is not empty: %s", pkgName) + } + } else if !os.IsNotExist(err) { + return "", util.ChildNewtError(err) + } + return pkgName, nil } diff --git a/newt/target_name_test.go b/newt/target_name_test.go new file mode 100644 index 00000000..f96cde9e --- /dev/null +++ b/newt/target_name_test.go @@ -0,0 +1,133 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package main + +import ( + "os" + "path/filepath" + "testing" +) + +// targetNameSnapshot returns contents of all files under the grandparent +// of dir, so that tests can verify nothing was written inside or near dir. +func targetNameSnapshot(t *testing.T, dir string) map[string]string { + t.Helper() + + files := map[string]string{} + err := filepath.Walk(filepath.Dir(filepath.Dir(dir)), + func(p string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if !info.IsDir() { + files[p] = readFile(t, p) + } + return nil + }) + if err != nil { + t.Fatal(err) + } + + return files +} + +func targetNameCheckUnchanged(t *testing.T, dir string, + before map[string]string) { + + t.Helper() + + after := targetNameSnapshot(t, dir) + for p, data := range after { + old, ok := before[p] + if !ok { + t.Errorf("file created: %s", p) + } else if old != data { + t.Errorf("file modified: %s", p) + } + } + for p := range before { + if _, ok := after[p]; !ok { + t.Errorf("file removed: %s", p) + } + } +} + +// Invalid or conflicting names passed to "target create" and "target copy" +// must be rejected without touching any files. +func TestTargetNameInvalid(t *testing.T) { + names := []string{ + "", + "/", + ".", + "..", + "x/..", + "../../x", + "targets/../x", + "/tmp/x", + "apps/app", + "success", + "targets/success", + } + + for _, name := range names { + for _, cmd := range [][]string{ + {"target", "create", name}, + {"target", "copy", "success", name}, + } { + projDir := copyToTemp(t, + filepath.Join("testdata", "syscfg_collision")) + before := targetNameSnapshot(t, projDir) + + out, err := newtCombinedOutput(projDir, nil, cmd...) + if err == nil { + t.Errorf("newt %q succeeded\n%s", cmd, out) + } + + targetNameCheckUnchanged(t, projDir, before) + } + } +} + +func TestTargetNameValid(t *testing.T) { + projDir := copyToTemp(t, filepath.Join("testdata", "syscfg_collision")) + + if err := os.MkdirAll(filepath.Join(projDir, "targets", "empty"), + 0755); err != nil { + t.Fatal(err) + } + + newtOutput(t, projDir, nil, "target", "create", "new") + newtOutput(t, projDir, nil, "target", "create", "apps/newtgt") + newtOutput(t, projDir, nil, "target", "copy", "success", "empty") + + for _, p := range []string{ + "targets/new/pkg.yml", + "targets/new/target.yml", + "apps/newtgt/pkg.yml", + "apps/newtgt/target.yml", + "targets/empty/pkg.yml", + "targets/empty/target.yml", + "targets/empty/syscfg.yml", + } { + if _, err := os.Stat(filepath.Join(projDir, p)); err != nil { + t.Error(err) + } + } +}
