check pdsc files in a pack and correct version compare (#574)

## Fixes
- check the position of pdsc files inside a pack for correctness
- corrected version compare with list --updatable and existing local
pack installations

## Changes
- search a pack for existing pdsc files
- check version of local installations before those in the index

## Checklist
<!-- Put an `x` in the boxes. All tasks must be completed and boxes
checked before merging. -->
- [ ] 🤖 This change is covered by unit tests (if applicable).
- [x] 🤹 Manual testing has been performed (if necessary).
- [x] 🛡️ Security impacts have been considered (if relevant).
- [x] 📖 Documentation updates are complete (if required).
- [x] 🧠 Third-party dependencies and TPIP updated (if required).

Author: bgn42

cmd/errors/errors.go

@@ -32,16 +32,18 @@ var (
 	ErrBadPackVersion = errors.New("bad pack version: cannot add a version with build metadata")
 
 	// Errors related to package content
-	ErrPdscFileNotFound      = errors.New("pdsc not found")
-	ErrPackNotInstalled      = errors.New("pack not installed")
-	ErrPdscEntryExists       = errors.New("pdsc already in index")
-	ErrPdscEntryNotFound     = errors.New("pdsc not found in index")
-	ErrEula                  = errors.New("user does not agree with the pack's license")
-	ErrExtractEula           = errors.New("user wants to extract embedded license only")
-	ErrLicenseNotFound       = errors.New("embedded license not found")
-	ErrPackRootNotFound      = errors.New("no CMSIS Pack Root directory specified. Either the environment CMSIS_PACK_ROOT needs to be set or the path specified using the command line option -R/--pack-root string")
-	ErrPackRootDoesNotExist  = errors.New("the specified CMSIS Pack Root directory does NOT exist! Please take a moment to review if the value is correct or create a new one via `cpackget init` command")
-	ErrPdscFileTooDeepInPack = errors.New("pdsc file is too deep in pack file")
+	ErrPdscFileNotFound        = errors.New("pdsc not found")
+	ErrPackNotInstalled        = errors.New("pack not installed")
+	ErrPdscEntryExists         = errors.New("pdsc already in index")
+	ErrPdscEntryNotFound       = errors.New("pdsc not found in index")
+	ErrEula                    = errors.New("user does not agree with the pack's license")
+	ErrExtractEula             = errors.New("user wants to extract embedded license only")
+	ErrLicenseNotFound         = errors.New("embedded license not found")
+	ErrPackRootNotFound        = errors.New("no CMSIS Pack Root directory specified. Either the environment CMSIS_PACK_ROOT needs to be set or the path specified using the command line option -R/--pack-root string")
+	ErrPackRootDoesNotExist    = errors.New("the specified CMSIS Pack Root directory does NOT exist! Please take a moment to review if the value is correct or create a new one via `cpackget init` command")
+	ErrPdscFileTooDeepInPack   = errors.New("pdsc file is too deep in pack file")
+	ErrMultiplePdscFilesInPack = errors.New("multiple pdsc files found in pack file, cannot determine which one to use. Please remove the extra pdsc files")
+	ErrPdscWrongName           = errors.New("pdsc file has wrong name, it should be <PackID>.pdsc")
 
 	// Errors related to network
 	ErrBadRequest            = errors.New("bad request")

cmd/installer/pack.go

@@ -212,78 +212,110 @@ func (p *PackType) fetch(timeout int) error {
 // to be installed.
 func (p *PackType) validate() error {
 	log.Debug("Validating pack")
+	var err error
 	myPdscFileName := p.PdscFileName()
+	var validPdscFiles []*zip.File
+
+	// Single pass: validate all files and collect valid PDSC files
 	for _, file := range p.zipReader.File {
 		ext := strings.ToLower(filepath.Ext(file.Name))
+
+		// Ensure all file paths do not contain ".."
+		if strings.Contains(file.Name, "..") {
+			if ext == PdscExtension {
+				log.Errorf("File %q invalid file path", file.Name)
+				return errs.ErrInvalidFilePath
+			} else {
+				return errs.ErrInsecureZipFileName
+			}
+		}
+
 		if ext == PdscExtension {
 			// Check if pack was compressed in a subfolder
 			subfoldersCount := strings.Count(file.Name, "/") + strings.Count(file.Name, "\\")
 			if subfoldersCount > 1 {
-				return errs.ErrPdscFileTooDeepInPack
-			} else if subfoldersCount == 1 {
-				p.Subfolder = filepath.Dir(file.Name)
-			}
+				err = errs.ErrPdscFileTooDeepInPack // save for later decision if error or warning
+			} else {
+				tmpFileName := filepath.Base(file.Name) // normalize file name
+				if !strings.EqualFold(tmpFileName, myPdscFileName) {
+					if err != nil {
+						log.Warnf("Pack %q contains an additional .pdsc file in a deeper subfolder, this may cause issues", p.path)
+					}
+					return fmt.Errorf("%q: %w", file.Name, errs.ErrPdscWrongName)
+				}
+				// Collect valid PDSC files for processing
+				validPdscFiles = append(validPdscFiles, file)
 
-			// Ensure the file path does not contain ".."
-			if strings.Contains(file.Name, "..") {
-				log.Errorf("File %q invalid file path", file.Name)
-				return errs.ErrInvalidFilePath
+				if subfoldersCount == 1 {
+					p.Subfolder = filepath.Dir(file.Name)
+				}
 			}
+		}
+	}
 
-			myPdscFileName = p.PackID() + filepath.Ext(file.Name)
+	if len(validPdscFiles) > 1 {
+		return errs.ErrMultiplePdscFilesInPack
+	}
 
-			// Read pack's pdsc
-			tmpPdscFileName := filepath.Join(os.TempDir(), utils.RandStringBytes(10))
-			defer os.RemoveAll(tmpPdscFileName)
+	if err != nil {
+		if len(validPdscFiles) > 0 {
+			log.Warnf("Pack %q contains an additional .pdsc file in a deeper subfolder, this may cause issues", p.path)
+		} else {
+			return err
+		}
+	}
 
-			if err := utils.SecureInflateFile(file, tmpPdscFileName, ""); err != nil {
-				return err
-			}
+	if len(validPdscFiles) == 0 {
+		log.Errorf("%q not found in %q", myPdscFileName, p.path)
+		return errs.ErrPdscFileNotFound
+	}
 
-			p.Pdsc = xml.NewPdscXML(filepath.Join(tmpPdscFileName, file.Name)) // #nosec
-			if err := p.Pdsc.Read(); err != nil {
-				return err
-			}
+	// Process the first valid PDSC file found
+	file := validPdscFiles[0]
 
-			// Sanity check: make sure the version being installed actually exists in the PDSC file
-			version := p.GetVersion()
-			latestVersion := p.Pdsc.LatestVersion()
+	// Read pack's pdsc
+	tmpPdscFileName := filepath.Join(os.TempDir(), utils.RandStringBytes(10))
+	defer os.RemoveAll(tmpPdscFileName)
 
-			log.Debugf("Making sure %s is the latest release in %s", version, myPdscFileName)
+	if err := utils.SecureInflateFile(file, tmpPdscFileName, ""); err != nil {
+		return err
+	}
 
-			if utils.SemverCompare(version, latestVersion) != 0 {
-				releaseTag := p.Pdsc.FindReleaseTagByVersion(version)
-				if releaseTag == nil {
-					log.Errorf("The pack's pdsc (%s) has no release tag matching version %q", myPdscFileName, version)
-					return errs.ErrPackVersionNotFoundInPdsc
-				}
+	p.Pdsc = xml.NewPdscXML(filepath.Join(tmpPdscFileName, file.Name)) // #nosec
+	if err := p.Pdsc.Read(); err != nil {
+		return err
+	}
 
-				log.Errorf("The latest release (%s) in pack's pdsc (%s) does not match pack version %q", latestVersion, myPdscFileName, version)
-				return errs.ErrPackVersionNotLatestReleasePdsc
-			}
+	// Sanity check: make sure the version being installed actually exists in the PDSC file
+	version := p.GetVersion()
+	latestVersion := p.Pdsc.LatestVersion()
 
-			p.Pdsc.FileName = file.Name
+	log.Debugf("Making sure %s is the latest release in %s", version, myPdscFileName)
 
-			pdscFileName := p.PdscFileName()                          // destination file name in .Download directory
-			pdscFilePath := filepath.Join(tmpPdscFileName, file.Name) // #nosec
-			newPdscFileName := p.PdscFileNameWithVersion()
+	if utils.SemverCompare(version, latestVersion) != 0 {
+		releaseTag := p.Pdsc.FindReleaseTagByVersion(version)
+		if releaseTag == nil {
+			log.Errorf("The pack's pdsc (%s) has no release tag matching version %q", myPdscFileName, version)
+			return errs.ErrPackVersionNotFoundInPdsc
+		}
 
-			if !p.IsPublic {
-				_ = utils.CopyFile(pdscFilePath, filepath.Join(Installation.LocalDir, pdscFileName))
-			}
+		log.Errorf("The latest release (%s) in pack's pdsc (%s) does not match pack version %q", latestVersion, myPdscFileName, version)
+		return errs.ErrPackVersionNotLatestReleasePdsc
+	}
 
-			_ = utils.CopyFile(pdscFilePath, filepath.Join(Installation.DownloadDir, newPdscFileName))
+	p.Pdsc.FileName = file.Name
 
-			return nil
-		} else {
-			if strings.Contains(file.Name, "..") {
-				return errs.ErrInsecureZipFileName
-			}
-		}
+	pdscFileName := p.PdscFileName()                          // destination file name in .Download directory
+	pdscFilePath := filepath.Join(tmpPdscFileName, file.Name) // #nosec
+	newPdscFileName := p.PdscFileNameWithVersion()
+
+	if !p.IsPublic {
+		_ = utils.CopyFile(pdscFilePath, filepath.Join(Installation.LocalDir, pdscFileName))
 	}
 
-	log.Errorf("%q not found in %q", myPdscFileName, p.path)
-	return errs.ErrPdscFileNotFound
+	_ = utils.CopyFile(pdscFilePath, filepath.Join(Installation.DownloadDir, newPdscFileName))
+
+	return nil
 }
 
 // purge removes all cached files matching the pattern derived from the PackType's

cmd/installer/root.go

@@ -1894,9 +1894,21 @@ func (p *PacksInstallationType) PackIsInstalled(pack *PackType, noLocal bool) (f
 		if latestVersion == "" {
 			log.Debugf("Could not find latest version for %q", pack.PackIDWithVersion())
 		} else {
+			log.Debugf("Checking for installed packs %s", utils.FormatVersions(pack.Version))
+			for _, version := range installedVersions {
+				log.Debugf("- checking against: %s", version)
+				if utils.SemverCompare(version, latestVersion) >= 0 {
+					found = true
+					return
+				}
+			}
 			packDir := filepath.Join(installationDir, latestVersion)
 			found = utils.DirExists(packDir)
-			pack.targetVersion = latestVersion
+			if !found && utils.SemverCompare(latestVersion, pack.Version) >= 0 {
+				pack.targetVersion = latestVersion
+			} else {
+				pack.targetVersion = pack.Version
+			}
 		}
 	}