Skip to content

Commit ae2a6ba

Browse files
Check for validity of the cached expanded APK more thoroughly (#1987)
Instead of just checking for the presence of a single file, this ensures that all active pieces of the expandapk structure are actually still valid. It checks that all necessary files exist and checks that cached file handles are actually the ones pointing to the files present in the filesystem.
1 parent 347c2ff commit ae2a6ba

File tree

4 files changed

+57
-2
lines changed

4 files changed

+57
-2
lines changed

internal/tarfs/tarfs.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,10 @@ func New(ra io.ReaderAt, size int64) (*FS, error) {
262262
return fsys, nil
263263
}
264264

265+
func (fsys *FS) UnderlyingReader() io.ReaderAt {
266+
return fsys.ra
267+
}
268+
265269
func (fsys *FS) Close() error {
266270
if fsys == nil {
267271
return nil

pkg/apk/apk/implementation.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1292,7 +1292,7 @@ func (c *apkCache) get(ctx context.Context, a *APK, pkg InstallablePackage) (*ex
12921292
// If we find a value in the cache, we should check to make sure the tar file it references still exists.
12931293
// If it references a non-existent file, we should act as though this was a cache miss and expand the
12941294
// APK again.
1295-
if _, err := os.Stat(result.exp.TarFile); os.IsNotExist(err) {
1295+
if !result.exp.IsValid() {
12961296
newValue := sync.OnceValue(fn)
12971297
c.onces.Store(u, newValue)
12981298
result = newValue()

pkg/apk/apk/implementation_test.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -756,11 +756,13 @@ func TestFetchPackage(t *testing.T) {
756756
t.Run("handle missing cache files when expanding APK", func(t *testing.T) {
757757
tmpDir := t.TempDir()
758758
a := prepLayout(t, tmpDir)
759+
759760
// Fill the cache
760761
exp, err := a.expandPackage(ctx, pkg)
761762
require.NoError(t, err, "unable to expand package")
762763
_, err = os.Stat(exp.TarFile)
763764
require.NoError(t, err, "unable to stat cached tar file")
765+
764766
// Delete the tar file from the cache
765767
require.NoError(t, os.Remove(exp.TarFile), "unable to delete cached tar file")
766768
_, err = os.Stat(exp.TarFile)
@@ -769,9 +771,22 @@ func TestFetchPackage(t *testing.T) {
769771
// Expand the package again, this should re-populate the cache.
770772
exp2, err := a.expandPackage(ctx, pkg)
771773
require.NoError(t, err, "unable to expandPackage after deleting cached tar file")
774+
_, err = os.Stat(exp2.TarFile)
775+
require.NoError(t, err, "unable to stat cached tar file")
776+
777+
// Delete and recreate the tar file from the cache (changing its inodes)
778+
bs, err := os.ReadFile(exp2.TarFile)
779+
require.NoError(t, err, "unable to read cached tar file")
780+
require.NoError(t, os.Remove(exp2.TarFile), "unable to delete cached tar file")
781+
require.NoError(t, os.WriteFile(exp2.TarFile, bs, 0o644), "unable to recreate cached tar file")
782+
783+
// Ensure that the underlying reader is different (i.e. we re-read the file)
784+
exp3, err := a.expandPackage(ctx, pkg)
785+
require.NoError(t, err, "unable to expandPackage after deleting and recreating cached tar file")
786+
require.NotEqual(t, exp2.TarFS.UnderlyingReader(), exp3.TarFS.UnderlyingReader())
772787

773788
// We should be able to read the APK contents
774-
rc, err := exp2.APK()
789+
rc, err := exp3.APK()
775790
require.NoError(t, err, "unable to get reader for APK()")
776791
_, err = io.ReadAll(rc)
777792
require.NoError(t, err, "unable to read APK contents")

pkg/apk/expandapk/expandapk.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,42 @@ func (m *multiReadCloser) Close() error {
209209
return errors.Join(errs...)
210210
}
211211

212+
// IsValid checks that the expanded APK is still valid by verifying that
213+
// the underlying files exist and that the file handles match the expected files.
214+
// Since this structure is heavily cached, this is useful to verify that the
215+
// cached data is still valid.
216+
func (a *APKExpanded) IsValid() bool {
217+
if f, ok := a.TarFS.UnderlyingReader().(*os.File); ok {
218+
// Verify that the file descriptor matches the expected file on disk.
219+
fdInfo, err := f.Stat()
220+
if err != nil {
221+
return false
222+
}
223+
224+
pathInfo, err := os.Stat(f.Name())
225+
if err != nil {
226+
return false
227+
}
228+
229+
if !os.SameFile(fdInfo, pathInfo) {
230+
return false
231+
}
232+
}
233+
234+
// Check that all the expected files exist.
235+
files := []string{a.ControlFile, a.PackageFile, a.TarFile}
236+
if a.SignatureFile != "" {
237+
files = append(files, a.SignatureFile)
238+
}
239+
for _, file := range files {
240+
if _, err := os.Stat(file); err != nil {
241+
return false
242+
}
243+
}
244+
245+
return true
246+
}
247+
212248
func (a *APKExpanded) Close() error {
213249
errs := []error{}
214250

0 commit comments

Comments
 (0)