From 01a10dad38bdec68a6647117fc8bde83a46c6682 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Apr 2026 20:01:59 -0500 Subject: [PATCH 1/5] fix(feature): add SHA-256 integrity verification for direct tar downloads (#89) Downloaded feature tarballs via HTTP had no integrity verification. Compute SHA-256 after download and store as a sidecar file; on cache hits, verify the cached tarball against the stored hash. Hash mismatch triggers re-download. Missing hash files (backward compat) log a warning and continue. --- pkg/devcontainer/feature/features.go | 101 ++++++++++++++---- .../feature/features_integrity_test.go | 74 +++++++++++++ 2 files changed, 152 insertions(+), 23 deletions(-) create mode 100644 pkg/devcontainer/feature/features_integrity_test.go diff --git a/pkg/devcontainer/feature/features.go b/pkg/devcontainer/feature/features.go index 7df768ffb..0fd47ab1e 100644 --- a/pkg/devcontainer/feature/features.go +++ b/pkg/devcontainer/feature/features.go @@ -283,6 +283,70 @@ func downloadLayer(img v1.Image, id, destFile string) error { return writeLayerToFile(data, destFile) } +// verifyCacheIntegrity checks a cached tarball against its stored +// SHA-256 sidecar. Returns true when the cache is safe to use. +func verifyCacheIntegrity(featureFolder, id string) bool { + hashFile := filepath.Join(featureFolder, "feature.sha256") + storedBytes, err := os.ReadFile(filepath.Clean(hashFile)) + if err != nil { + log.Warnf( + "No integrity hash for cached feature (backward compat): featureId=%s", + id, + ) + return true + } + + tarball := filepath.Join(featureFolder, "feature.tgz") + computed, err := hash.File(tarball) + if err != nil { + log.Errorf("Failed to hash cached tarball: error=%v, featureId=%s", err, id) + return false + } + + if computed != strings.TrimSpace(string(storedBytes)) { + log.Errorf( + "Integrity check failed for cached feature: featureId=%s", + id, + ) + return false + } + + log.Debugf("Integrity check passed for cached feature: featureId=%s", id) + return true +} + +// storeIntegrityHash computes and persists the SHA-256 of a downloaded tarball. +func storeIntegrityHash(featureFolder, tarballPath, id string) { + computed, err := hash.File(tarballPath) + if err != nil { + log.Errorf("Failed to compute tarball hash: error=%v, featureId=%s", err, id) + return + } + + hashFile := filepath.Join(featureFolder, "feature.sha256") + if err := os.WriteFile(hashFile, []byte(computed), 0o600); err != nil { + log.Errorf("Failed to write hash sidecar: error=%v, featureId=%s", err, id) + return + } + + log.Infof("Feature tarball integrity: featureId=%s, sha256=%s", id, computed) +} + +func extractTarball(downloadFile, dest string) error { + file, err := os.Open(downloadFile) + if err != nil { + return fmt.Errorf("open tarball: %w", err) + } + defer func() { _ = file.Close() }() + + if err := extract.Extract(file, dest); err != nil { + _ = os.RemoveAll(dest) + return fmt.Errorf("extract folder: %w", err) + } + + return nil +} + func processDirectTarFeature( id string, httpHeaders map[string]string, @@ -299,20 +363,24 @@ func processDirectTarFeature( ) } - // feature already exists? featureFolder, err := getFeaturesTempFolder(id) if err != nil { return "", fmt.Errorf("resolve feature cache dir: %w", err) } featureExtractedFolder := filepath.Join(featureFolder, "extracted") - _, err = os.Stat(featureExtractedFolder) - if err == nil && !forceDownload { - log.Debugf("direct tar feature already cached: folder=%s", featureExtractedFolder) - return featureExtractedFolder, nil + + // Check cache — verify integrity if present. + _, statErr := os.Stat(featureExtractedFolder) + if statErr == nil && !forceDownload { + if verifyCacheIntegrity(featureFolder, id) { + log.Debugf("direct tar feature already cached: folder=%s", featureExtractedFolder) + return featureExtractedFolder, nil + } + _ = os.RemoveAll(featureFolder) } - // download feature tarball + // Download feature tarball. downloadFile := filepath.Join(featureFolder, "feature.tgz") err = downloadFeatureFromURL(id, downloadFile, httpHeaders) if err != nil { @@ -320,24 +388,11 @@ func processDirectTarFeature( return "", err } - // extract file - file, err := os.Open(downloadFile) - if err != nil { - log.Errorf("failed to open downloaded tarball: error=%v, file=%s", err, downloadFile) - return "", err - } - defer func() { _ = file.Close() }() + storeIntegrityHash(featureFolder, downloadFile, id) - // extract tar.gz - err = extract.Extract(file, featureExtractedFolder) - if err != nil { - log.Errorf( - "failed to extract tarball: error=%v, destination=%s", - err, - featureExtractedFolder, - ) - _ = os.RemoveAll(featureExtractedFolder) - return "", fmt.Errorf("extract folder: %w", err) + if err := extractTarball(downloadFile, featureExtractedFolder); err != nil { + log.Errorf("failed to extract tarball: error=%v, featureId=%s", err, id) + return "", err } log.Infof( diff --git a/pkg/devcontainer/feature/features_integrity_test.go b/pkg/devcontainer/feature/features_integrity_test.go new file mode 100644 index 000000000..591a9fa8c --- /dev/null +++ b/pkg/devcontainer/feature/features_integrity_test.go @@ -0,0 +1,74 @@ +package feature + +import ( + "os" + "path/filepath" + "testing" + + "github.com/devsy-org/devsy/pkg/hash" + "github.com/stretchr/testify/suite" +) + +type IntegrityTestSuite struct { + suite.Suite +} + +func TestIntegrityTestSuite(t *testing.T) { + suite.Run(t, new(IntegrityTestSuite)) +} + +// createTestTarball writes a fake tarball file and returns its path. +func createTestTarball(dir string) (string, error) { + tarball := filepath.Join(dir, "feature.tgz") + return tarball, os.WriteFile(tarball, []byte("fake-tarball-content"), 0o600) +} + +func (s *IntegrityTestSuite) TestStoreIntegrityHash_WritesCorrectSidecar() { + dir := s.T().TempDir() + tarball, err := createTestTarball(dir) + s.Require().NoError(err) + + storeIntegrityHash(dir, tarball, "test-feature") + + hashFile := filepath.Join(dir, "feature.sha256") + stored, err := os.ReadFile(hashFile) + s.Require().NoError(err) + + expected, err := hash.File(tarball) + s.Require().NoError(err) + s.Equal(expected, string(stored)) +} + +func (s *IntegrityTestSuite) TestVerifyCacheIntegrity_ValidHash() { + dir := s.T().TempDir() + tarball, err := createTestTarball(dir) + s.Require().NoError(err) + + computed, err := hash.File(tarball) + s.Require().NoError(err) + + hashFile := filepath.Join(dir, "feature.sha256") + s.Require().NoError(os.WriteFile(hashFile, []byte(computed), 0o600)) + + s.True(verifyCacheIntegrity(dir, "test-feature")) +} + +func (s *IntegrityTestSuite) TestVerifyCacheIntegrity_CorruptedHash() { + dir := s.T().TempDir() + _, err := createTestTarball(dir) + s.Require().NoError(err) + + hashFile := filepath.Join(dir, "feature.sha256") + s.Require().NoError(os.WriteFile(hashFile, []byte("bad-hash"), 0o600)) + + s.False(verifyCacheIntegrity(dir, "test-feature")) +} + +func (s *IntegrityTestSuite) TestVerifyCacheIntegrity_MissingHashFile() { + dir := s.T().TempDir() + _, err := createTestTarball(dir) + s.Require().NoError(err) + + // No .sha256 file — backward compat: should return true. + s.True(verifyCacheIntegrity(dir, "test-feature")) +} From bfafb7f123f70a3f75fc19503a833a60e2cd46db Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Apr 2026 20:08:57 -0500 Subject: [PATCH 2/5] fix: resolve gosec G304 in extractTarball and integrity test Use filepath.Clean on file paths passed to os.Open and os.ReadFile to satisfy gosec G304 (potential file inclusion via variable). --- pkg/devcontainer/feature/features.go | 2 +- pkg/devcontainer/feature/features_integrity_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/devcontainer/feature/features.go b/pkg/devcontainer/feature/features.go index 0fd47ab1e..2edb67846 100644 --- a/pkg/devcontainer/feature/features.go +++ b/pkg/devcontainer/feature/features.go @@ -333,7 +333,7 @@ func storeIntegrityHash(featureFolder, tarballPath, id string) { } func extractTarball(downloadFile, dest string) error { - file, err := os.Open(downloadFile) + file, err := os.Open(filepath.Clean(downloadFile)) if err != nil { return fmt.Errorf("open tarball: %w", err) } diff --git a/pkg/devcontainer/feature/features_integrity_test.go b/pkg/devcontainer/feature/features_integrity_test.go index 591a9fa8c..f1de31e73 100644 --- a/pkg/devcontainer/feature/features_integrity_test.go +++ b/pkg/devcontainer/feature/features_integrity_test.go @@ -31,7 +31,7 @@ func (s *IntegrityTestSuite) TestStoreIntegrityHash_WritesCorrectSidecar() { storeIntegrityHash(dir, tarball, "test-feature") hashFile := filepath.Join(dir, "feature.sha256") - stored, err := os.ReadFile(hashFile) + stored, err := os.ReadFile(filepath.Clean(hashFile)) s.Require().NoError(err) expected, err := hash.File(tarball) From ce701b037241c265e12016d2c46cf5e86d02450e Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Apr 2026 20:30:05 -0500 Subject: [PATCH 3/5] test(e2e): add integrity verification test for direct tar features Extend the up-features e2e suite with a test that exercises the SHA-256 integrity verification path. The test downloads a feature tarball via HTTP, then creates a second workspace with the same feature URL and asserts that the cached tarball is reused (integrity check passes, no re-download). --- e2e/tests/up-features/up_features.go | 89 ++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/e2e/tests/up-features/up_features.go b/e2e/tests/up-features/up_features.go index fe4a103de..ac87ccc89 100644 --- a/e2e/tests/up-features/up_features.go +++ b/e2e/tests/up-features/up_features.go @@ -107,6 +107,95 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features", "suite framework.ExpectNoError(err) }, ginkgo.SpecTimeout(framework.GetTimeout())) + ginkgo.It( + "direct tar feature uses cached download with integrity verification", + func(ctx context.Context) { + server := ghttp.NewServer() + ginkgo.DeferCleanup(server.Close) + + tempDir1, err := framework.CopyToTempDir( + "tests/up-features/testdata/docker-features-http-headers", + ) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir1) + + tempDir2, err := framework.CopyToTempDir( + "tests/up-features/testdata/docker-features-http-headers", + ) + framework.ExpectNoError(err) + ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir2) + + featureArchiveFilePath := path.Join(tempDir1, "devcontainer-feature-hello.tgz") + featureFiles := []string{ + path.Join(tempDir1, "devcontainer-feature.json"), + path.Join(tempDir1, "install.sh"), + } + err = createTarGzArchive(featureArchiveFilePath, featureFiles) + framework.ExpectNoError(err) + + for _, dir := range []string{tempDir1, tempDir2} { + devContainerFileBuf, err := os.ReadFile(path.Join(dir, ".devcontainer.json")) + framework.ExpectNoError(err) + + output := strings.ReplaceAll( + string(devContainerFileBuf), + "#{server_url}", + server.URL(), + ) + // #nosec G306 -- test file, permissive mode is acceptable. + err = os.WriteFile(path.Join(dir, ".devcontainer.json"), []byte(output), 0o644) + framework.ExpectNoError(err) + } + + respHeader := http.Header{} + respHeader.Set( + "Content-Disposition", + "attachment; filename=devcontainer-feature-hello.tgz", + ) + + featureArchiveFileBuf, err := os.ReadFile(featureArchiveFilePath) + framework.ExpectNoError(err) + + server.AppendHandlers( + ghttp.CombineHandlers( + ghttp.VerifyRequest("GET", "/devcontainer-feature-hello.tgz"), + ghttp.RespondWith(http.StatusOK, featureArchiveFileBuf, respHeader), + ), + ) + + f := framework.NewDefaultFramework(initialDir + "/bin") + _ = f.DevsyProviderDelete(ctx, "docker") + + err = f.DevsyProviderAdd(ctx, "docker") + framework.ExpectNoError(err) + + err = f.DevsyProviderUse(ctx, "docker") + framework.ExpectNoError(err) + + // First workspace: downloads feature, stores .sha256 sidecar + wsName1 := filepath.Base(tempDir1) + ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName1) + + err = f.DevsyUp(ctx, tempDir1) + framework.ExpectNoError(err) + + // Delete first workspace; feature cache persists across deletions + err = f.DevsyWorkspaceDelete(ctx, wsName1) + framework.ExpectNoError(err) + + // Second workspace: cache hit, integrity verification passes, no download + wsName2 := filepath.Base(tempDir2) + ginkgo.DeferCleanup(f.DevsyWorkspaceDelete, wsName2) + + err = f.DevsyUp(ctx, tempDir2) + framework.ExpectNoError(err) + + // Only one HTTP request was made — proves cache was reused with passing integrity + gomega.Expect(server.ReceivedRequests()).To(gomega.HaveLen(1)) + }, + ginkgo.SpecTimeout(framework.GetTimeout()), + ) + ginkgo.It("should install with lifecycle hooks", func(ctx context.Context) { f, err := setupDockerProvider(initialDir+"/bin", "docker") framework.ExpectNoError(err) From 50973f4ff342be7e28533b5b879b61128cb75d50 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Apr 2026 20:37:38 -0500 Subject: [PATCH 4/5] fix: resolve gosec G304 in e2e integrity verification test Use filepath.Clean on variable paths passed to os.ReadFile to satisfy gosec G304 (potential file inclusion via variable). --- e2e/tests/up-features/up_features.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/e2e/tests/up-features/up_features.go b/e2e/tests/up-features/up_features.go index ac87ccc89..1e0dc2831 100644 --- a/e2e/tests/up-features/up_features.go +++ b/e2e/tests/up-features/up_features.go @@ -134,7 +134,8 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features", "suite framework.ExpectNoError(err) for _, dir := range []string{tempDir1, tempDir2} { - devContainerFileBuf, err := os.ReadFile(path.Join(dir, ".devcontainer.json")) + devContainerFile := filepath.Clean(path.Join(dir, ".devcontainer.json")) + devContainerFileBuf, err := os.ReadFile(devContainerFile) framework.ExpectNoError(err) output := strings.ReplaceAll( @@ -153,7 +154,7 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features", "suite "attachment; filename=devcontainer-feature-hello.tgz", ) - featureArchiveFileBuf, err := os.ReadFile(featureArchiveFilePath) + featureArchiveFileBuf, err := os.ReadFile(filepath.Clean(featureArchiveFilePath)) framework.ExpectNoError(err) server.AppendHandlers( From 71c36923c4f8fe894211d175fd48fb49ab74ce49 Mon Sep 17 00:00:00 2001 From: Samuel K Date: Thu, 23 Apr 2026 21:21:09 -0500 Subject: [PATCH 5/5] fix(e2e): restore cwd between CopyToTempDir calls in integrity test CopyToTempDir changes the working directory, so the second copy could not resolve its relative testdata path. Chdir back to initialDir between the two copy calls. --- e2e/tests/up-features/up_features.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/e2e/tests/up-features/up_features.go b/e2e/tests/up-features/up_features.go index 1e0dc2831..61dd79232 100644 --- a/e2e/tests/up-features/up_features.go +++ b/e2e/tests/up-features/up_features.go @@ -119,6 +119,10 @@ var _ = ginkgo.Describe("testing up command", ginkgo.Label("up-features", "suite framework.ExpectNoError(err) ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir1) + // CopyToTempDir changes cwd; restore so the second copy resolves its relative path. + err = os.Chdir(initialDir) + framework.ExpectNoError(err) + tempDir2, err := framework.CopyToTempDir( "tests/up-features/testdata/docker-features-http-headers", )