test: drive utils coverage to 100% by yi-nuo426 · Pull Request #955 · meshery/meshkit

yi-nuo426 · 2026-03-31T22:46:12Z

Summary

add focused coverage tests for utils, utils/csv, and utils/walker
add minimal package-private seams plus tightly coupled fixes for hard-to-hit archive, CUE, file, git, and SVG paths in utils
validate the root utils package at 100.0% statement coverage

Testing

go test ./utils ./utils/csv ./utils/walker
go test -coverprofile=/tmp/utils.cover ./utils
make test

- add focused coverage tests for utils, csv, and walker - add minimal utils seams and tightly coupled bug fixes for hard-to-hit paths - validate 100 percent coverage for the root utils package and rerun make test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

welcome · 2026-03-31T22:46:16Z

Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, you can find updates in the #github-notifications channel in the community Slack.
Be sure to double-check that you have signed your commits. Here are instructions for making signing an implicit activity while performing a commit.

simihablo

Additional tests. :thu

welcome · 2026-03-31T22:49:22Z

Thanks for your contribution to Meshery! 🎉

Meshery Logo
Join the community, if you haven't yet and please leave a ⭐ star on the project. 😄

gemini-code-assist

Code Review

This pull request significantly refactors various utility functions to enhance testability and error handling, primarily by introducing mockable interfaces and internal variables for file I/O, CUE operations, and external services. It also adds comprehensive unit tests for CSV parsing, error constructors, and general utilities. Critical feedback was provided regarding a Zip Slip vulnerability and potential file descriptor leaks in the unarchiving logic, along with a resource leak in the file writing utility where a file handle remains open upon a write error.

gemini-code-assist · 2026-03-31T22:49:54Z


 		if file.FileInfo().IsDir() {
-			err := os.Mkdir(file.Name, file.Mode())
+			err := os.MkdirAll(filePath, file.Mode())


This code is vulnerable to a "Zip Slip" attack. The filePath is constructed by joining the destination path with the file name from the archive without validating that the resulting path remains within the destination directory. A malicious archive could contain file names with path traversal elements (e.g., ../../etc/passwd) to overwrite sensitive files on the system. You should validate that the cleaned filePath has the destination path as a prefix.

gemini-code-assist · 2026-03-31T22:49:54Z

+		defer func() {
+			_ = fd.Close()
+		}()


Using defer inside a loop can lead to resource exhaustion, specifically file descriptor leaks. Deferred functions are only executed when the surrounding function returns, not at the end of each loop iteration. Since this loop processes all files in a ZIP archive, it could exceed the process's file descriptor limit if the archive contains many files. This also applies to the openedFile.Close() defer on line 110. Consider wrapping the loop body in a function literal or closing the files explicitly.

gemini-code-assist · 2026-03-31T22:49:54Z

 // WriteToFile writes the given content to the given file path
 func WriteToFile(path string, content string) error {
-	file, err := os.Create(path)
+	file, err := createWritableFile(path)


The file handle opened by createWritableFile is not closed if file.WriteString(content) returns an error at line 337. This results in a resource leak. Consider adding a defer file.Close() immediately after the error check for createWritableFile to ensure the file is closed on all exit paths.

Copilot

Pull request overview

This PR focuses on driving statement coverage of the utils package (and related utils/csv and utils/walker) to 100% by adding targeted tests and introducing small package-private seams to make hard-to-reach error paths testable.

Changes:

Added extensive coverage tests for utils, utils/csv, and utils/walker.
Introduced package-private seams (function variables/interfaces) in utils to allow deterministic testing of error paths (archive, CUE, file IO, Google Sheets, SVG).
Made a few behavior tweaks to improve robustness and enable coverage (e.g., skipping blank CSV rows in Git(), safer ErrUnmarshalInvalid formatting).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
utils/walker/walker_test.go	Adds coverage tests for local directory walking and git/GitHub walkers.
utils/utils.go	Adds IO/marshal/tar seams and improves YAML write panic handling; updates some callsites to use seams.
utils/utils_coverage_test.go	Adds focused “coverage-driving” tests across many `utils` helpers and error branches.
utils/unarchive.go	Adds seams for zip/tar extraction and adjusts zip extraction behavior.
utils/svg_utils.go	Adds seam for XML token encoding to test failure paths.
utils/google.go	Adds seam for Sheets service construction to test error handling.
utils/git.go	Adjusts parsing to skip blank rows when reading git version data.
utils/error.go	Avoids panic when `reflect.Type` is nil in `ErrUnmarshalInvalid`.
utils/error_constructors_test.go	Adds tests asserting error constructors expose expected error codes.
utils/cue.go	Adds seams for formatting/compiling/lookup to test error paths deterministically.
utils/csv/csv_test.go	Adds coverage tests for CSV parser read/parse and error wrapping behavior.

Comments suppressed due to low confidence (1)

utils/utils.go:339

If WriteString fails, WriteToFile returns without closing the newly created file, leaking a file descriptor. Ensure the file is closed on all paths (e.g., defer Close() immediately after creation, or explicitly close before returning the write error).

	file, err := createWritableFile(path)
	if err != nil {
		return ErrCreateFile(err, path)
	}

	_, err = file.WriteString(content)
	if err != nil {
		return ErrWriteFile(err, path)
	}
	// Close the file to save the changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T22:52:42Z

 		filePath := filepath.Join(path, file.Name)

 		if file.FileInfo().IsDir() {
-			err := os.Mkdir(file.Name, file.Mode())
+			err := os.MkdirAll(filePath, file.Mode())
 			if err != nil {


ExtractZip is vulnerable to ZipSlip/path traversal: file.Name can contain absolute paths or .. segments, and filepath.Join(path, file.Name) can escape the intended destination directory. Sanitize and validate the resulting path stays within path before creating directories/files.

Copilot · 2026-03-31T22:52:42Z

 	for _, file := range zipReader.File {
-
-		fd, err := file.Open()
-		defer func() {
-			_ = fd.Close()
-		}()
-
+		fd, err := openZipEntry(file)
 		if err != nil {
 			return ErrExtractZip(err, path)
 		}
+		defer func() {
+			_ = fd.Close()
+		}()


defer fd.Close() is executed inside the zip iteration loop, so all zip entry readers stay open until ExtractZip returns. On large archives this can exhaust file descriptors; close fd at the end of each iteration (and avoid defer inside the loop).

Copilot · 2026-03-31T22:52:43Z

+			openedFile, err := os.OpenFile(filePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, file.Mode())
 			if err != nil {
 				return ErrExtractZip(err, path)
 			}
 			defer func() {
 				_ = openedFile.Close()
 			}()
+			_, err = copyZipBuffer(openedFile, fd, buffer)
+			if err != nil {
+				return ErrExtractZip(err, path)
+			}


defer openedFile.Close() is inside the zip iteration loop; if many files are extracted, the destination file descriptors remain open until function exit. Close each file explicitly after copyZipBuffer (or scope the defer to a helper func) to avoid FD exhaustion.

Copilot · 2026-03-31T22:52:43Z

 	defer func() { _ = encoder.Close() }()
+	defer func() {
+		if recovered := recover(); recovered != nil {
+			switch value := recovered.(type) {
+			case error:
+				err = ErrMarshal(value)
+			default:
+				err = ErrMarshal(fmt.Errorf("%v", value))
+			}
+		}
+	}()


The added recover() in WriteYamlToFile will convert all panics (including runtime.Error like nil-pointer dereferences) into ErrMarshal, which can mask serious bugs and make failures harder to diagnose. Consider only recovering expected marshal panics (or re-panicking for runtime.Error) so genuine programming errors still crash loudly.

Copilot · 2026-03-31T22:52:43Z

 			if err != nil {
 				return ErrExtractTarXZ(err, path)
 			}
-			if _, err = io.Copy(outFile, tarReader); err != nil {
+			if _, err = copyTarContent(outFile, tarReader); err != nil {
 				return ErrExtractTarXZ(err, path)
 			}
 			_ = outFile.Close()


ExtractTarGz still builds output paths from header.Name (via filepath.Join(path, header.Name) in this loop) without validating for absolute paths or .. segments, which enables TarSlip/path traversal writes outside path. Add path sanitization/validation before MkdirAll/Create so extracted files stay within the destination directory.

Copilot AI review requested due to automatic review settings March 31, 2026 22:46

simihablo approved these changes Mar 31, 2026

View reviewed changes

leecalcote merged commit 35e099c into master Mar 31, 2026
4 of 5 checks passed

leecalcote deleted the copilot/utils-coverage-tests branch March 31, 2026 22:49

Copilot started reviewing on behalf of yi-nuo426 March 31, 2026 22:49 View session

gemini-code-assist Bot reviewed Mar 31, 2026

View reviewed changes

Copilot AI reviewed Mar 31, 2026

View reviewed changes

Conversation

yi-nuo426 commented Mar 31, 2026

Summary

Testing

Uh oh!

welcome Bot commented Mar 31, 2026

Uh oh!

simihablo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

welcome Bot commented Mar 31, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants