Skip to content

Merging to release-5.10: [TT-14814] fix bundle loading issue (#7436) #7452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 14, 2025
Merged

Merging to release-5.10: [TT-14814] fix bundle loading issue (#7436) #7452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ac5a324
[TT-14814] fix bundle loading issue (#7436)
pvormste Oct 14, 2025
0da18d6
fix conflict
pvormste Oct 14, 2025
6b4dc10
finish conflict resolving
pvormste Oct 14, 2025
aa044cb
remove sleep from failing test
pvormste Oct 14, 2025
7ef0378
Merge branch 'release-5.10' into merge/release-5.10/a9909715512222e96…
pvormste Oct 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 86 additions & 49 deletions gateway/coprocess_bundle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,6 @@
package gateway

import (
"path"
"strings"
"time"

"github.com/cenk/backoff"

"github.com/sirupsen/logrus"

"archive/zip"
"bytes"
"crypto/md5"
Expand All @@ -21,10 +13,18 @@
"io/ioutil"
"net/http"
"net/url"
"os"
"path"
"path/filepath"
"strings"
"time"

"github.com/cenk/backoff"
"github.com/spf13/afero"

"github.com/sirupsen/logrus"

"github.com/TykTechnologies/goverify"

"github.com/TykTechnologies/tyk/apidef"
"github.com/TykTechnologies/tyk/internal/sanitize"
)
Expand All @@ -44,8 +44,8 @@
Gw *Gateway `json:"-"`
}

// Verify performs a signature verification on the bundle file.
func (b *Bundle) Verify() error {
// Verify performs signature verification on the bundle file.
func (b *Bundle) Verify(bundleFs afero.Fs) error {
log.WithFields(logrus.Fields{
"prefix": "main",
}).Info("----> Verifying bundle: ", b.Spec.CustomMiddlewareBundle)
Expand Down Expand Up @@ -74,7 +74,7 @@
for _, f := range b.Manifest.FileList {
extractedPath := filepath.Join(b.Path, f)

f, err := os.Open(extractedPath)
f, err := bundleFs.Open(extractedPath)
if err != nil {
return err
}
Expand Down Expand Up @@ -137,6 +137,7 @@

// FileBundleGetter is a BundleGetter for testing.
type FileBundleGetter struct {
Fs afero.Fs
URL string
InsecureSkipVerify bool
}
Expand Down Expand Up @@ -168,7 +169,7 @@

// Get mocks an HTTP(S) GET request.
func (g *FileBundleGetter) Get() ([]byte, error) {
return os.ReadFile(strings.TrimPrefix(g.URL, "file://"))
return afero.ReadFile(g.Fs, strings.TrimPrefix(g.URL, "file://"))
}

// BundleSaver is an interface used by bundle saver structures.
Expand All @@ -177,50 +178,72 @@
}

// ZipBundleSaver is a BundleSaver for ZIP files.
type ZipBundleSaver struct{}
type ZipBundleSaver struct {
Fs afero.Fs
}

// Save implements the main method of the BundleSaver interface. It makes use of archive/zip.
func (ZipBundleSaver) Save(bundle *Bundle, bundlePath string, spec *APISpec) error {
func (z *ZipBundleSaver) Save(bundle *Bundle, bundlePath string, _ *APISpec) error {
buf := bytes.NewReader(bundle.Data)
reader, err := zip.NewReader(buf, int64(len(bundle.Data)))
if err != nil {
return err
}

for _, f := range reader.File {
if err := sanitize.ZipFilePath(f.Name, bundlePath); err != nil {
if err := z.extractFile(f, bundlePath); err != nil {
return err
}
}

destPath := filepath.Join(bundlePath, f.Name)
return nil
}

if f.FileHeader.Mode().IsDir() {
if err := os.Mkdir(destPath, 0700); err != nil {
return err
}
continue
}
rc, err := f.Open()
if err != nil {
return err
}
newFile, err := os.Create(destPath)
if err != nil {
return err
}
if _, err = io.Copy(newFile, rc); err != nil {
return err
func (z *ZipBundleSaver) extractFile(f *zip.File, bundlePath string) error {
if err := sanitize.ZipFilePath(f.Name, bundlePath); err != nil {
return err
}

destPath := filepath.Join(bundlePath, f.Name)

if f.FileHeader.Mode().IsDir() {
return z.Fs.Mkdir(destPath, 0700)
}

rc, err := f.Open()
if err != nil {
return err
}
defer func() {
if err := rc.Close(); err != nil {
log.WithFields(logrus.Fields{
"prefix": "main",
}).WithError(err).Error("Couldn't close file")
}
rc.Close()
}()

newFile, err := z.Fs.Create(destPath)
if err != nil {
return err
}

defer func() {
if err := newFile.Close(); err != nil {
return err
log.WithFields(logrus.Fields{
"prefix": "main",
}).WithError(err).Error("Couldn't close file")

Check failure on line 234 in gateway/coprocess_bundle.go

View check run for this annotation

probelabs / Visor: quality

logic Issue 

Errors from `newFile.Close()` are logged but not propagated. A failure to close a file (e.g., due to a disk write error during flush) will not be reported to the caller, potentially leading to corrupted or incomplete bundle files being treated as valid.
Raw output 
Handle the errors from `Close()` calls explicitly and return them. This ensures that I/O failures during file closing are properly propagated and handled.
}
}()

if _, err = io.Copy(newFile, rc); err != nil {
return err
}

return nil
}

// FetchBundle will fetch a given bundle, using the right BundleGetter. The first argument is the bundle name, the base bundle URL will be used as prefix.
func (gw *Gateway) FetchBundle(spec *APISpec) (Bundle, error) {
func (gw *Gateway) FetchBundle(bundleFs afero.Fs, spec *APISpec) (Bundle, error) {
bundle := Bundle{Gw: gw}
var err error

Expand Down Expand Up @@ -256,6 +279,7 @@
}
case "file":
getter = &FileBundleGetter{
Fs: bundleFs,
URL: bundleURL,
InsecureSkipVerify: gw.GetConfig().BundleInsecureSkipVerify,
}
Expand Down Expand Up @@ -295,28 +319,30 @@
}

// saveBundle will save a bundle to the disk, see ZipBundleSaver methods for reference.
func saveBundle(bundle *Bundle, destPath string, spec *APISpec) error {
func saveBundle(bundleFs afero.Fs, bundle *Bundle, destPath string, spec *APISpec) error {
bundleFormat := "zip"

var bundleSaver BundleSaver

// TODO: use enums?
switch bundleFormat {
case "zip":
bundleSaver = ZipBundleSaver{}
bundleSaver = &ZipBundleSaver{
Fs: bundleFs,
}
}

return bundleSaver.Save(bundle, destPath, spec)
}

// loadBundleManifest will parse the manifest file and return the bundle parameters.
func loadBundleManifest(bundle *Bundle, spec *APISpec, skipVerification bool) error {
func loadBundleManifest(bundleFs afero.Fs, bundle *Bundle, spec *APISpec, skipVerification bool) error {
log.WithFields(logrus.Fields{
"prefix": "main",
}).Info("----> Loading bundle: ", spec.CustomMiddlewareBundle)

manifestPath := filepath.Join(bundle.Path, "manifest.json")
f, err := os.Open(manifestPath)
f, err := bundleFs.Open(manifestPath)
if err != nil {
return err
}
Expand All @@ -333,7 +359,7 @@
return nil
}

if err := bundle.Verify(); err != nil {
if err := bundle.Verify(bundleFs); err != nil {
log.WithFields(logrus.Fields{
"prefix": "main",
}).Info("----> Bundle verification failed: ", spec.CustomMiddlewareBundle)
Expand All @@ -357,8 +383,18 @@
return fmt.Sprintf("%x", bundleNameHash.Sum(nil)), nil
}

// loadBundle wraps the load and save steps, it will return if an error occurs at any point.
// loadBundle configures the gateway to load a custom middleware bundle based on the provided API specification.
// It verifies the existence, integrity, and configuration of the bundle, applying it to the spec if validation succeeds.
// Returns an error if the bundle cannot be loaded, validated, or is disabled in the spec.
func (gw *Gateway) loadBundle(spec *APISpec) error {
return gw.loadBundleWithFs(spec, afero.NewOsFs())
}

// loadBundleWithFs loads and validates a middleware bundle for the given API specification using the provided filesystem.
// It operates only if required settings like CustomMiddlewareBundle and BundleBaseURL are configured in the API spec.
// The method handles bundle fetching, saving, and manifest validation.
// Returns an error if the bundle cannot be fetched, saved, or its manifest cannot be verified successfully.
func (gw *Gateway) loadBundleWithFs(spec *APISpec, bundleFs afero.Fs) error {
if gw.GetConfig().ManagementNode {
return nil
}
Expand All @@ -378,7 +414,7 @@

// Skip if the bundle destination path already exists.
// The bundle exists, load and return:
if _, err := os.Stat(destPath); err == nil {
if _, err := bundleFs.Stat(destPath); err == nil {
log.WithFields(logrus.Fields{
"prefix": "main",
}).Info("Loading existing bundle: ", spec.CustomMiddlewareBundle)
Expand All @@ -388,13 +424,14 @@
Path: destPath,
Spec: spec,
Gw: gw,
}

err = loadBundleManifest(&bundle, spec, true)
err = loadBundleManifest(bundleFs, &bundle, spec, false)
if err != nil {
log.WithFields(logrus.Fields{
"prefix": "main",
}).Info("----> Couldn't load bundle: ", spec.CustomMiddlewareBundle, " ", err)

Check warning on line 433 in gateway/coprocess_bundle.go

View check run for this annotation

probelabs / Visor: quality

logic Issue 

When an existing bundle fails manifest verification, the error is correctly returned, but the invalid bundle directory and its contents are left on the filesystem. This could lead to confusion or potential issues on subsequent gateway restarts if the invalid artifacts are not cleaned up.
Raw output 
To ensure a clean state and prevent the use of invalid artifacts, remove the bundle directory if manifest validation fails for an existing bundle. This behavior would be consistent with how newly fetched bundles are handled on verification failure.
return err
}

log.WithFields(logrus.Fields{
Expand All @@ -410,16 +447,16 @@
"prefix": "main",
}).Info("----> Fetching Bundle: ", spec.CustomMiddlewareBundle)

bundle, err := gw.FetchBundle(spec)
bundle, err := gw.FetchBundle(bundleFs, spec)
if err != nil {
return bundleError(spec, err, "Couldn't fetch bundle")
}

if err := os.MkdirAll(destPath, 0700); err != nil {
if err := bundleFs.MkdirAll(destPath, 0700); err != nil {
return bundleError(spec, err, "Couldn't create bundle directory")
}

if err := saveBundle(&bundle, destPath, spec); err != nil {
if err := saveBundle(bundleFs, &bundle, destPath, spec); err != nil {
return bundleError(spec, err, "Couldn't save bundle")
}

Expand All @@ -430,10 +467,10 @@
// Set the destination path:
bundle.Path = destPath

if err := loadBundleManifest(&bundle, spec, false); err != nil {
if err := loadBundleManifest(bundleFs, &bundle, spec, false); err != nil {
bundleError(spec, err, "Couldn't load bundle")

if removeErr := os.RemoveAll(bundle.Path); removeErr != nil {
if removeErr := bundleFs.RemoveAll(bundle.Path); removeErr != nil {
bundleError(spec, removeErr, "Couldn't remove bundle")
}
return err
Expand Down
Loading
Loading