Skip to content

Commit 51db4da

Browse files
committed
fix: preserve nested archives which fail to extract
Signed-off-by: egibs <20933572+egibs@users.noreply.github.com>
1 parent 86e0d54 commit 51db4da

2 files changed

Lines changed: 117 additions & 3 deletions

File tree

pkg/action/archive_test.go

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
package action
55

66
import (
7+
"archive/tar"
78
"bytes"
9+
"compress/gzip"
810
"context"
911
"io/fs"
1012
"os"
@@ -592,6 +594,114 @@ func TestScanConflictingArchiveFiles(t *testing.T) {
592594
}
593595
}
594596

597+
// createBrokenNestedArchive creates a tar.gz file containing a nested
598+
// file with an archive extension whose content is valid gzip but invalid tar.
599+
func createBrokenNestedArchive(t *testing.T, dir string) string {
600+
t.Helper()
601+
602+
outPath := filepath.Join(dir, "outer.tar.gz")
603+
f, err := os.Create(outPath)
604+
if err != nil {
605+
t.Fatalf("failed to create outer archive: %v", err)
606+
}
607+
defer f.Close()
608+
609+
gw := gzip.NewWriter(f)
610+
defer gw.Close()
611+
tw := tar.NewWriter(gw)
612+
defer tw.Close()
613+
614+
var innerBuf bytes.Buffer
615+
innerGw := gzip.NewWriter(&innerBuf)
616+
if _, err := innerGw.Write(bytes.Repeat([]byte("A"), 1024)); err != nil {
617+
t.Fatalf("failed to write inner gzip data: %v", err)
618+
}
619+
if err := innerGw.Close(); err != nil {
620+
t.Fatalf("failed to close inner gzip writer: %v", err)
621+
}
622+
623+
innerData := innerBuf.Bytes()
624+
if err := tw.WriteHeader(&tar.Header{
625+
Name: "bad_nested.tar.gz",
626+
Mode: 0o600,
627+
Size: int64(len(innerData)),
628+
}); err != nil {
629+
t.Fatalf("failed to write tar header: %v", err)
630+
}
631+
if _, err := tw.Write(innerData); err != nil {
632+
t.Fatalf("failed to write tar data: %v", err)
633+
}
634+
635+
return outPath
636+
}
637+
638+
// TestNestedFailureRetention verifies that when a nested archive
639+
// extraction fails with ExitExtraction=false (default), the original nested archive
640+
// file is retained in the extraction directory for scanning rather than being deleted.
641+
func TestNestedFailureRetention(t *testing.T) {
642+
t.Parallel()
643+
644+
tmpDir, err := os.MkdirTemp("", "nested-fail-retain-*")
645+
if err != nil {
646+
t.Fatalf("failed to create temp dir: %v", err)
647+
}
648+
defer os.RemoveAll(tmpDir)
649+
650+
outerArchive := createBrokenNestedArchive(t, tmpDir)
651+
652+
ctx := context.Background()
653+
cfg := malcontent.Config{ExitExtraction: false}
654+
655+
extractDir, err := archive.ExtractArchiveToTempDir(ctx, cfg, outerArchive)
656+
if err != nil {
657+
t.Fatalf("ExtractArchiveToTempDir should not fail with ExitExtraction=false, got: %v", err)
658+
}
659+
defer os.RemoveAll(extractDir)
660+
661+
// The nested archive file must still exist so it can be scanned as a regular file
662+
found := false
663+
err = filepath.WalkDir(extractDir, func(_ string, d os.DirEntry, err error) error {
664+
if err != nil {
665+
return err
666+
}
667+
if d.Name() == "bad_nested.tar.gz" {
668+
found = true
669+
}
670+
return nil
671+
})
672+
if err != nil {
673+
t.Fatalf("failed to walk extraction directory: %v", err)
674+
}
675+
if !found {
676+
t.Fatal("nested archive file was deleted after extraction failure but should be retained for scanning")
677+
}
678+
}
679+
680+
// TestNestedFailureRetentionError verifies that when ExitExtraction=true,
681+
// a nested archive extraction failure propagates as an error.
682+
func TestNestedFailureRetentionError(t *testing.T) {
683+
t.Parallel()
684+
685+
tmpDir, err := os.MkdirTemp("", "nested-fail-exit-*")
686+
if err != nil {
687+
t.Fatalf("failed to create temp dir: %v", err)
688+
}
689+
defer os.RemoveAll(tmpDir)
690+
691+
outerArchive := createBrokenNestedArchive(t, tmpDir)
692+
693+
ctx := context.Background()
694+
cfg := malcontent.Config{ExitExtraction: true}
695+
696+
extractDir, err := archive.ExtractArchiveToTempDir(ctx, cfg, outerArchive)
697+
if extractDir != "" {
698+
defer os.RemoveAll(extractDir)
699+
}
700+
if err == nil {
701+
t.Fatal("ExtractArchiveToTempDir should return error with ExitExtraction=true for nested archives which cannot be extracted")
702+
}
703+
}
704+
595705
func TestIsValidPath(t *testing.T) {
596706
tmpRoot, err := os.MkdirTemp("", "isValidPath-*")
597707
if err != nil {

pkg/archive/archive.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,13 +194,17 @@ func extractNestedArchive(ctx context.Context, c malcontent.Config, d string, f
194194
if c.ExitExtraction {
195195
return fmt.Errorf("failed to extract archive: %w", err)
196196
}
197-
logger.Debugf("ignoring extraction error for %s: %s", f, err.Error())
197+
logger.Warnf("extraction failed for %s, retaining archive for scanning: %s", f, err.Error())
198198
}
199199

200200
extracted.Store(f, true)
201201

202-
if err := os.Remove(fullPath); err != nil {
203-
return fmt.Errorf("failed to remove archive file: %w", err)
202+
// only attempt to remove the archive file if we don't encounter an extraction error
203+
// any archives which cannot be extracted will be scanned like non-archive files
204+
if err == nil {
205+
if err := os.Remove(fullPath); err != nil {
206+
return fmt.Errorf("failed to remove archive file: %w", err)
207+
}
204208
}
205209

206210
entries, err := os.ReadDir(d)

0 commit comments

Comments
 (0)