Skip to content

Commit 4cdb9fa

Browse files
divyansh42pramodbindal
authored andcommitted
Fix go lint errors
Signed-off-by: divyansh42 <diagrawa@redhat.com>
1 parent 204789c commit 4cdb9fa

File tree

3 files changed

+31
-15
lines changed

3 files changed

+31
-15
lines changed

internal/provider/blob/blob.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"log"
77
"net/url"
88
"os"
9+
"strings"
910

1011
"github.com/openshift-pipelines/tekton-caches/internal/tar"
1112
"gocloud.dev/blob"
@@ -41,8 +42,14 @@ func init() {
4142
queryParams = os.Getenv("BLOB_QUERY_PARAMS")
4243
}
4344

45+
// sanitizeLog strips newline/carriage-return characters from s to prevent log injection.
46+
func sanitizeLog(s string) string {
47+
s = strings.ReplaceAll(s, "\n", "")
48+
return strings.ReplaceAll(s, "\r", "")
49+
}
50+
4451
func Fetch(ctx context.Context, url url.URL, folder string) error {
45-
log.Printf("Downloading cache from %s to %s", url.String(), folder)
52+
log.Printf("Downloading cache from %s to %s", sanitizeLog(url.String()), sanitizeLog(folder)) //nolint:gosec
4653
file, err := os.CreateTemp("", cacheFile)
4754
if err != nil {
4855
log.Printf("error creating tar file: %s", err)
@@ -83,7 +90,7 @@ func Fetch(ctx context.Context, url url.URL, folder string) error {
8390
}
8491

8592
func Upload(ctx context.Context, url url.URL, folder string) error {
86-
log.Printf("Uploading cache to %s from %s", url.String(), folder)
93+
log.Printf("Uploading cache to %s from %s", sanitizeLog(url.String()), sanitizeLog(folder)) //nolint:gosec
8794
file, err := os.CreateTemp("", cacheFile)
8895
if err != nil {
8996
return err

internal/tar/file-utils.go

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
package tar
1+
package tar //nolint:revive
22

33
import (
44
"archive/tar"
@@ -88,8 +88,14 @@ func Compress(source, target string) error {
8888
return nil
8989
}
9090

91+
// sanitizeLog strips newline/carriage-return characters from s to prevent log injection.
92+
func sanitizeLog(s string) string {
93+
s = strings.ReplaceAll(s, "\n", "")
94+
return strings.ReplaceAll(s, "\r", "")
95+
}
96+
9197
func ExtractTarGz(file *os.File, targetDir string) error {
92-
log.Printf("Extracting tar.gz...%s", file.Name())
98+
log.Printf("Extracting tar.gz...%s", sanitizeLog(file.Name())) //nolint:gosec
9399
gz, err := gzip.NewReader(file)
94100
if err != nil {
95101
return fmt.Errorf("error creating gzip reader: %w", err)
@@ -101,7 +107,7 @@ func ExtractTarGz(file *os.File, targetDir string) error {
101107
}
102108

103109
func ExtractTar(file *os.File, targetDir string) error {
104-
log.Printf("Extracting tar... %s", file.Name())
110+
log.Printf("Extracting tar... %s", sanitizeLog(file.Name())) //nolint:gosec
105111
return extract(tar.NewReader(file), targetDir)
106112
}
107113

@@ -129,16 +135,16 @@ func extract(tr *tar.Reader, targetDir string) error {
129135

130136
switch header.Typeflag {
131137
case tar.TypeDir:
132-
// Ensure directory exists
133-
if err := os.MkdirAll(path, os.ModePerm); err != nil {
138+
// Ensure directory exists; path is validated by safePath above.
139+
if err := os.MkdirAll(path, os.ModePerm); err != nil { //nolint:gosec
134140
return err
135141
}
136142
// Change the directory permission so that all users can create/update files inside.
137-
if err := os.Chmod(path, os.ModePerm); path != baseDir && err != nil {
143+
if err := os.Chmod(path, os.ModePerm); path != baseDir && err != nil { //nolint:gosec
138144
return fmt.Errorf("failed to change permissions of %s: %w", path, err)
139145
}
140146
case tar.TypeReg:
141-
outFile, err := os.Create(path)
147+
outFile, err := os.Create(path) //nolint:gosec
142148
if err != nil {
143149
return fmt.Errorf("error while creating file %s: %w", path, err)
144150
}
@@ -171,9 +177,10 @@ func extract(tr *tar.Reader, targetDir string) error {
171177
}
172178

173179
func safePath(basePath, targetPath string) (string, error) {
174-
if strings.Contains(targetPath, "..") {
175-
return "", fmt.Errorf("target path contains '..': %s", targetPath)
180+
combinedPath := filepath.Clean(filepath.Join(basePath, targetPath))
181+
rel, err := filepath.Rel(basePath, combinedPath)
182+
if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
183+
return "", fmt.Errorf("target path %q escapes base directory", targetPath)
176184
}
177-
combinedPath := filepath.Join(basePath, targetPath)
178185
return combinedPath, nil
179186
}

internal/tar/file_utils_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1-
package tar
1+
package tar_test
22

33
import (
44
"os"
55
"path/filepath"
66
"testing"
7+
8+
cachetar "github.com/openshift-pipelines/tekton-caches/internal/tar"
79
)
810

911
func TestComressAndExtract(t *testing.T) {
@@ -29,7 +31,7 @@ func TestComressAndExtract(t *testing.T) {
2931

3032
// Test Compress
3133
tarFile := filepath.Join(tempDir, "test.tar.gz")
32-
err = Compress(testDir, tarFile)
34+
err = cachetar.Compress(testDir, tarFile)
3335
if err != nil {
3436
t.Fatalf("Compress failed: %v", err)
3537
}
@@ -52,7 +54,7 @@ func TestComressAndExtract(t *testing.T) {
5254
}
5355
defer tarFileHandle.Close()
5456

55-
err = ExtractTarGz(tarFileHandle, extractDir)
57+
err = cachetar.ExtractTarGz(tarFileHandle, extractDir)
5658
if err != nil {
5759
t.Fatalf("ExtractTarGz failed: %v", err)
5860
}

0 commit comments

Comments
 (0)