fix(kubectl-plugin/log): validate header.Mode and release fds per iteration

[SAY-5]

Fixes #4695.

What

ClusterLogOptions.downloadRayLogFiles in kubectl-plugin/pkg/cmd/log/log.go had three issues called out in the bug:

1. gosec G115 (integer overflow). tar.Header.Mode is int64, os.FileMode is uint32. The existing overflow check printed a warning but then fell through to os.FileMode(header.Mode) anyway, so a crafted tar header could still apply unexpected mode bits. Now we continue when the mode is out of range and cast explicitly through uint32 so the G115 diagnostic is honoured:

   if header.Mode < 0 || header.Mode > math.MaxUint32 {
       fmt.Fprintf(..., "file mode %d outside of acceptable range, skipping file %s\n", header.Mode, header.Name)
       continue
   }
   outFile, err := os.OpenFile(localFilePath, os.O_CREATE|os.O_RDWR, os.FileMode(uint32(header.Mode)))

2. File-descriptor leak. defer outFile.Close() sat inside the extraction loop, so every opened file stayed open until downloadRayLogFiles returned. Extracting a large log tar can hit the too many open files ulimit. The copy is now wrapped in an IIFE so the defer fires at the end of each iteration:

   copyErr := func() error {
       defer outFile.Close()
       for {
           ...
       }
   }()

3. Error message quality. out side → outside, accceptable → acceptable, include the offending filename, add a newline.

Why

(1) is a security improvement (gosec was right), (2) is a reliability improvement for large log tars, and (3) is housekeeping that falls out of touching the surrounding block anyway.

Signed off per DCO.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Reviewed by Cursor Bugbot for commit 145d519. Configure here.}

[SAY-5]

Thanks @cursor for catching this — that was a real bug. continue inside the switch continues the outer for but skips tarReader.Next() below, so a bad header.Mode would spin forever.

Fixed by using break to exit just the switch, which lets execution fall through to the tarReader.Next() that advances to the next entry. Also left an inline comment explaining the subtlety so future readers don't regress it. Force-pushed.

[hango880623]

Since this changes the behavior in log.go, it’d be great to add a couple of tests in log_test.go to cover the new cases:

• skipping files with invalid Mode values (e.g. > math.MaxUint32)

• making sure extraction keeps going after hitting one of those

• maybe updating the test helper to make it easier to create tar files with different modes / lots of entries

[SAY-5]

Hi @hango880623, thanks for the review.

Pushed both pieces in 6a86fc4d:

1. More flexible test helper — extracted buildFakeTarFile([]fakeTarEntry) so tests can drop arbitrary entries (modes, ordering, etc.) directly. The existing createFakeTarFile() is preserved as a thin wrapper so the original TestDownloadRayLogFiles keeps running unchanged.

2. Regression test — TestDownloadRayLogFiles_SkipsInvalidMode builds a tar with three entries: a valid one (good_before.txt), an out-of-range mode entry (math.MaxUint32 + 1 — the smallest int64 that fails the bounds check), and a third valid one (good_after.txt). It asserts:

   • downloadRayLogFiles returns no error;
   • good_before.txt and good_after.txt land on disk;
   • bad_mode.txt is skipped (not written).

   That pins both halves of the fix — the bounds rejection and the break-instead-of-continue change so a bad entry doesn't loop forever.

go test -run TestDownloadRayLogFiles -v ./kubectl-plugin/pkg/cmd/log/ → 2/2 passing.