Skip to content

Commit 8cf011a

Browse files
JAORMXclaude
andcommitted
Improve test coverage from 69% to 84%
Refactor ssh/client.go to inject a remoteSession interface for testability, then add ~70 tests across 8 packages covering security-critical tar extraction, lock contention, DNS interception, process lifecycle edge cases, and egress policy validation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 645f2b3 commit 8cf011a

11 files changed

Lines changed: 1819 additions & 16 deletions

File tree

image/pull_test.go

Lines changed: 344 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,3 +519,347 @@ func TestPullWithFetcher_NilFetcher(t *testing.T) {
519519
require.Error(t, err)
520520
assert.Contains(t, err.Error(), "pull image")
521521
}
522+
523+
// ---------------------------------------------------------------------------
524+
// extractHardlink tests
525+
// ---------------------------------------------------------------------------
526+
527+
func TestExtractHardlink_ValidLink(t *testing.T) {
528+
t.Parallel()
529+
530+
dst := t.TempDir()
531+
532+
entries := []tarEntry{
533+
{name: "original.txt", typeflag: tar.TypeReg, mode: 0o644, content: "hello"},
534+
{name: "link.txt", typeflag: tar.TypeLink, mode: 0o644, linkname: "original.txt"},
535+
}
536+
buf := createTarBuffer(t, entries)
537+
538+
err := extractTar(buf, dst)
539+
require.NoError(t, err)
540+
541+
origInfo, err := os.Lstat(filepath.Join(dst, "original.txt"))
542+
require.NoError(t, err)
543+
linkInfo, err := os.Lstat(filepath.Join(dst, "link.txt"))
544+
require.NoError(t, err)
545+
546+
origStat := origInfo.Sys().(*syscall.Stat_t)
547+
linkStat := linkInfo.Sys().(*syscall.Stat_t)
548+
assert.Equal(t, origStat.Ino, linkStat.Ino, "hardlink should share the same inode")
549+
}
550+
551+
func TestExtractHardlink_SourceOutsideRootfs(t *testing.T) {
552+
t.Parallel()
553+
554+
dst := t.TempDir()
555+
556+
entries := []tarEntry{
557+
{name: "escape.txt", typeflag: tar.TypeLink, mode: 0o644, linkname: "../../etc/passwd"},
558+
}
559+
buf := createTarBuffer(t, entries)
560+
561+
err := extractTar(buf, dst)
562+
require.Error(t, err)
563+
assert.Contains(t, err.Error(), "hardlink")
564+
assert.Contains(t, err.Error(), "outside rootfs")
565+
}
566+
567+
func TestExtractHardlink_SourceIsSymlink(t *testing.T) {
568+
t.Parallel()
569+
570+
dst := t.TempDir()
571+
572+
// Pre-create a symlink as the source.
573+
err := os.Symlink("nonexistent", filepath.Join(dst, "sym"))
574+
require.NoError(t, err)
575+
576+
entries := []tarEntry{
577+
{name: "link.txt", typeflag: tar.TypeLink, mode: 0o644, linkname: "sym"},
578+
}
579+
buf := createTarBuffer(t, entries)
580+
581+
err = extractTar(buf, dst)
582+
require.Error(t, err)
583+
assert.Contains(t, err.Error(), "refusing hardlink to symlink")
584+
}
585+
586+
func TestExtractHardlink_SourceIsDirectory(t *testing.T) {
587+
t.Parallel()
588+
589+
dst := t.TempDir()
590+
591+
// Pre-create a directory as the source.
592+
err := os.Mkdir(filepath.Join(dst, "mydir"), 0o755)
593+
require.NoError(t, err)
594+
595+
entries := []tarEntry{
596+
{name: "link.txt", typeflag: tar.TypeLink, mode: 0o644, linkname: "mydir"},
597+
}
598+
buf := createTarBuffer(t, entries)
599+
600+
err = extractTar(buf, dst)
601+
require.Error(t, err)
602+
assert.Contains(t, err.Error(), "refusing hardlink to non-regular file")
603+
}
604+
605+
func TestExtractHardlink_SourceNotExtracted(t *testing.T) {
606+
t.Parallel()
607+
608+
dst := t.TempDir()
609+
610+
entries := []tarEntry{
611+
{name: "link.txt", typeflag: tar.TypeLink, mode: 0o644, linkname: "does-not-exist.txt"},
612+
}
613+
buf := createTarBuffer(t, entries)
614+
615+
err := extractTar(buf, dst)
616+
require.Error(t, err)
617+
assert.Contains(t, err.Error(), "stat hardlink source")
618+
}
619+
620+
func TestExtractHardlink_TargetIsExistingSymlink(t *testing.T) {
621+
t.Parallel()
622+
623+
dst := t.TempDir()
624+
625+
// Create the real file that will be the hardlink source.
626+
err := os.WriteFile(filepath.Join(dst, "original.txt"), []byte("data"), 0o644)
627+
require.NoError(t, err)
628+
629+
// Pre-create a symlink at the target location.
630+
err = os.Symlink("original.txt", filepath.Join(dst, "link.txt"))
631+
require.NoError(t, err)
632+
633+
entries := []tarEntry{
634+
{name: "link.txt", typeflag: tar.TypeLink, mode: 0o644, linkname: "original.txt"},
635+
}
636+
buf := createTarBuffer(t, entries)
637+
638+
err = extractTar(buf, dst)
639+
require.Error(t, err)
640+
assert.Contains(t, err.Error(), "refusing to write through symlink")
641+
}
642+
643+
// ---------------------------------------------------------------------------
644+
// extractSymlink edge-case tests
645+
// ---------------------------------------------------------------------------
646+
647+
func TestExtractSymlink_AbsoluteEscapeAttempt(t *testing.T) {
648+
t.Parallel()
649+
650+
dst := t.TempDir()
651+
652+
entries := []tarEntry{
653+
{name: "escape", typeflag: tar.TypeSymlink, mode: 0o777, linkname: "/../../../etc/passwd"},
654+
}
655+
buf := createTarBuffer(t, entries)
656+
657+
err := extractTar(buf, dst)
658+
require.Error(t, err)
659+
assert.Contains(t, err.Error(), "points outside rootfs")
660+
}
661+
662+
func TestExtractSymlink_RelativeEscapeAttempt(t *testing.T) {
663+
t.Parallel()
664+
665+
dst := t.TempDir()
666+
667+
entries := []tarEntry{
668+
{name: "escape", typeflag: tar.TypeSymlink, mode: 0o777, linkname: "../../../../../../etc/passwd"},
669+
}
670+
buf := createTarBuffer(t, entries)
671+
672+
err := extractTar(buf, dst)
673+
require.Error(t, err)
674+
assert.Contains(t, err.Error(), "points outside rootfs")
675+
}
676+
677+
func TestExtractSymlink_ReplacesExistingFile(t *testing.T) {
678+
t.Parallel()
679+
680+
dst := t.TempDir()
681+
682+
// Extract a regular file, then a symlink at the same path.
683+
entries := []tarEntry{
684+
{name: "target.txt", typeflag: tar.TypeReg, mode: 0o644, content: "real content"},
685+
{name: "overwrite", typeflag: tar.TypeReg, mode: 0o644, content: "old"},
686+
{name: "overwrite", typeflag: tar.TypeSymlink, mode: 0o777, linkname: "target.txt"},
687+
}
688+
buf := createTarBuffer(t, entries)
689+
690+
err := extractTar(buf, dst)
691+
require.NoError(t, err)
692+
693+
// "overwrite" should now be a symlink to "target.txt".
694+
linkTarget, err := os.Readlink(filepath.Join(dst, "overwrite"))
695+
require.NoError(t, err)
696+
assert.Equal(t, "target.txt", linkTarget)
697+
698+
data, err := os.ReadFile(filepath.Join(dst, "overwrite"))
699+
require.NoError(t, err)
700+
assert.Equal(t, "real content", string(data))
701+
}
702+
703+
func TestExtractSymlink_RefusesToReplaceDirectory(t *testing.T) {
704+
t.Parallel()
705+
706+
dst := t.TempDir()
707+
708+
// Create a directory first, then attempt to replace it with a symlink.
709+
entries := []tarEntry{
710+
{name: "mydir/", typeflag: tar.TypeDir, mode: 0o755},
711+
{name: "mydir", typeflag: tar.TypeSymlink, mode: 0o777, linkname: "somewhere"},
712+
}
713+
buf := createTarBuffer(t, entries)
714+
715+
err := extractTar(buf, dst)
716+
require.Error(t, err)
717+
assert.Contains(t, err.Error(), "refusing to replace directory with symlink")
718+
}
719+
720+
// ---------------------------------------------------------------------------
721+
// mkdirAllNoSymlink tests
722+
// ---------------------------------------------------------------------------
723+
724+
func TestMkdirAllNoSymlink_SymlinkInPath(t *testing.T) {
725+
t.Parallel()
726+
727+
base := t.TempDir()
728+
729+
// Create base/a as a symlink to /tmp.
730+
err := os.Symlink(os.TempDir(), filepath.Join(base, "a"))
731+
require.NoError(t, err)
732+
733+
err = mkdirAllNoSymlink(base, filepath.Join(base, "a", "b"), 0o755)
734+
require.Error(t, err)
735+
assert.Contains(t, err.Error(), "refusing to traverse symlink")
736+
}
737+
738+
func TestMkdirAllNoSymlink_NonDirInPath(t *testing.T) {
739+
t.Parallel()
740+
741+
base := t.TempDir()
742+
743+
// Create a regular file where a directory is expected.
744+
err := os.WriteFile(filepath.Join(base, "notdir"), []byte("file"), 0o644)
745+
require.NoError(t, err)
746+
747+
err = mkdirAllNoSymlink(base, filepath.Join(base, "notdir", "child"), 0o755)
748+
require.Error(t, err)
749+
assert.Contains(t, err.Error(), "path component is not a directory")
750+
}
751+
752+
func TestMkdirAllNoSymlink_TargetOutsideBase(t *testing.T) {
753+
t.Parallel()
754+
755+
base := t.TempDir()
756+
757+
err := mkdirAllNoSymlink(base, "/tmp/outside", 0o755)
758+
require.Error(t, err)
759+
assert.Contains(t, err.Error(), "invalid target directory")
760+
}
761+
762+
func TestMkdirAllNoSymlink_TargetEqualsBase(t *testing.T) {
763+
t.Parallel()
764+
765+
base := t.TempDir()
766+
767+
// Target equals base should be a no-op (returns nil).
768+
err := mkdirAllNoSymlink(base, base, 0o755)
769+
require.NoError(t, err)
770+
}
771+
772+
// ---------------------------------------------------------------------------
773+
// validateNoSymlinkLeaf tests (table-driven)
774+
// ---------------------------------------------------------------------------
775+
776+
func TestValidateNoSymlinkLeaf(t *testing.T) {
777+
t.Parallel()
778+
779+
base := t.TempDir()
780+
781+
// Set up fixtures.
782+
symPath := filepath.Join(base, "sym")
783+
err := os.Symlink("nonexistent", symPath)
784+
require.NoError(t, err)
785+
786+
dirPath := filepath.Join(base, "dir")
787+
err = os.Mkdir(dirPath, 0o755)
788+
require.NoError(t, err)
789+
790+
filePath := filepath.Join(base, "file")
791+
err = os.WriteFile(filePath, []byte("data"), 0o644)
792+
require.NoError(t, err)
793+
794+
nonexistentPath := filepath.Join(base, "nonexistent")
795+
796+
tests := []struct {
797+
name string
798+
target string
799+
wantErr bool
800+
errSubstr string
801+
}{
802+
{name: "existing symlink", target: symPath, wantErr: true, errSubstr: "refusing to write through symlink"},
803+
{name: "existing directory", target: dirPath, wantErr: true, errSubstr: "refusing to overwrite directory with file"},
804+
{name: "existing regular file", target: filePath, wantErr: false},
805+
{name: "non-existent path", target: nonexistentPath, wantErr: false},
806+
}
807+
808+
for _, tt := range tests {
809+
t.Run(tt.name, func(t *testing.T) {
810+
t.Parallel()
811+
812+
err := validateNoSymlinkLeaf(tt.target)
813+
if tt.wantErr {
814+
require.Error(t, err)
815+
assert.Contains(t, err.Error(), tt.errSubstr)
816+
} else {
817+
require.NoError(t, err)
818+
}
819+
})
820+
}
821+
}
822+
823+
// ---------------------------------------------------------------------------
824+
// Other extractTar tests
825+
// ---------------------------------------------------------------------------
826+
827+
func TestExtractTar_EmptyArchive(t *testing.T) {
828+
t.Parallel()
829+
830+
// Create an empty tar archive (no entries).
831+
var buf bytes.Buffer
832+
tw := tar.NewWriter(&buf)
833+
err := tw.Close()
834+
require.NoError(t, err)
835+
836+
dst := t.TempDir()
837+
err = extractTar(&buf, dst)
838+
require.Error(t, err)
839+
assert.Contains(t, err.Error(), "empty or contains no valid entries")
840+
}
841+
842+
func TestExtractTar_UnsupportedEntryType(t *testing.T) {
843+
t.Parallel()
844+
845+
entries := []tarEntry{
846+
// FIFO (unsupported) -- should be skipped.
847+
{name: "myfifo", typeflag: tar.TypeFifo, mode: 0o644},
848+
// Regular file -- should be extracted.
849+
{name: "good.txt", typeflag: tar.TypeReg, mode: 0o644, content: "valid"},
850+
}
851+
buf := createTarBuffer(t, entries)
852+
853+
dst := t.TempDir()
854+
err := extractTar(buf, dst)
855+
require.NoError(t, err)
856+
857+
// FIFO should not exist.
858+
_, err = os.Lstat(filepath.Join(dst, "myfifo"))
859+
assert.True(t, os.IsNotExist(err), "FIFO should not have been extracted")
860+
861+
// Regular file should exist.
862+
data, err := os.ReadFile(filepath.Join(dst, "good.txt"))
863+
require.NoError(t, err)
864+
assert.Equal(t, "valid", string(data))
865+
}

0 commit comments

Comments
 (0)