From 43da580817854d83d107d99175f441a7c8a2c6ff Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 19 Jun 2026 16:13:49 +0200 Subject: [PATCH 01/20] feat: draft accessing bin store --- cmd/chisel/cmd_cut.go | 15 ++ cmd/chisel/main.go | 2 + internal/slicer/slicer.go | 2 + internal/store/export_test.go | 20 ++ internal/store/log.go | 53 +++++ internal/store/store.go | 274 ++++++++++++++++++++++ internal/store/store_test.go | 422 ++++++++++++++++++++++++++++++++++ internal/store/suite_test.go | 13 ++ 8 files changed, 801 insertions(+) create mode 100644 internal/store/export_test.go create mode 100644 internal/store/log.go create mode 100644 internal/store/store.go create mode 100644 internal/store/store_test.go create mode 100644 internal/store/suite_test.go diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 35c81a79a..a37967312 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -8,6 +8,7 @@ import ( "github.com/jessevdk/go-flags" "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/store" "github.com/canonical/chisel/internal/cache" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" @@ -121,9 +122,23 @@ func (cmd *cmdCut) Execute(args []string) error { } } + stores := make(map[string]store.Store) + for _, storeInfo := range release.Stores { + openStore, err := store.Open(&store.Options{ + Arch: cmd.Arch, + CacheDir: cache.DefaultDir("chisel"), + }) + if err != nil { + return err + } + stores[storeInfo.Name] = openStore + break + } + err = slicer.Run(&slicer.RunOptions{ Selection: selection, Archives: archives, + Stores: stores, TargetDir: cmd.RootDir, }) return err diff --git a/cmd/chisel/main.go b/cmd/chisel/main.go index 569c07ab9..0450e901c 100644 --- a/cmd/chisel/main.go +++ b/cmd/chisel/main.go @@ -16,6 +16,7 @@ import ( "github.com/canonical/chisel/internal/deb" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" + "github.com/canonical/chisel/internal/store" "github.com/canonical/chisel/internal/tarball" //"github.com/canonical/chisel/internal/logger" ) @@ -328,6 +329,7 @@ func run() error { deb.SetLogger(log.Default()) setup.SetLogger(log.Default()) slicer.SetLogger(log.Default()) + store.SetLogger(log.Default()) tarball.SetLogger(log.Default()) SetLogger(log.Default()) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index fcc16d673..426012de1 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -16,6 +16,7 @@ import ( "github.com/klauspost/compress/zstd" "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/store" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" @@ -28,6 +29,7 @@ const manifestMode fs.FileMode = 0644 type RunOptions struct { Selection *setup.Selection Archives map[string]archive.Archive + Stores map[string]store.Store TargetDir string } diff --git a/internal/store/export_test.go b/internal/store/export_test.go new file mode 100644 index 000000000..b4c64aadb --- /dev/null +++ b/internal/store/export_test.go @@ -0,0 +1,20 @@ +package store + +import "net/http" + +var ( + ValidateDownloadURL = validateDownloadURL + StagingEnvVar = stagingEnvVar +) + +func SetHTTPDo(fn func(req *http.Request) (*http.Response, error)) (restore func()) { + saved := httpDo + httpDo = fn + return func() { httpDo = saved } +} + +func SetBulkDo(fn func(req *http.Request) (*http.Response, error)) (restore func()) { + saved := bulkDo + bulkDo = fn + return func() { bulkDo = saved } +} diff --git a/internal/store/log.go b/internal/store/log.go new file mode 100644 index 000000000..d8c11a443 --- /dev/null +++ b/internal/store/log.go @@ -0,0 +1,53 @@ +package store + +import ( + "fmt" + "sync" +) + +// Avoid importing the log type information unnecessarily. There's a small cost +// associated with using an interface rather than the type. Depending on how +// often the logger is plugged in, it would be worth using the type instead. +type log_Logger interface { + Output(calldepth int, s string) error +} + +var globalLoggerLock sync.Mutex +var globalLogger log_Logger +var globalDebug bool + +// Specify the *log.Logger object where log messages should be sent to. +func SetLogger(logger log_Logger) { + globalLoggerLock.Lock() + globalLogger = logger + globalLoggerLock.Unlock() +} + +// Enable the delivery of debug messages to the logger. Only meaningful +// if a logger is also set. +func SetDebug(debug bool) { + globalLoggerLock.Lock() + globalDebug = debug + globalLoggerLock.Unlock() +} + +// logf sends to the logger registered via SetLogger the string resulting +// from running format and args through Sprintf. +func logf(format string, args ...any) { + globalLoggerLock.Lock() + defer globalLoggerLock.Unlock() + if globalLogger != nil { + globalLogger.Output(2, fmt.Sprintf(format, args...)) + } +} + +// debugf sends to the logger registered via SetLogger the string resulting +// from running format and args through Sprintf, but only if debugging was +// enabled via SetDebug. +func debugf(format string, args ...any) { + globalLoggerLock.Lock() + defer globalLoggerLock.Unlock() + if globalDebug && globalLogger != nil { + globalLogger.Output(2, fmt.Sprintf(format, args...)) + } +} diff --git a/internal/store/store.go b/internal/store/store.go new file mode 100644 index 000000000..c169ed3c2 --- /dev/null +++ b/internal/store/store.go @@ -0,0 +1,274 @@ +package store + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "os" + "strings" + "time" + + "github.com/canonical/chisel/internal/cache" + "github.com/canonical/chisel/internal/deb" +) + +// Store provides access to bin packages from the Snapcraft store API. +type Store interface { + Fetch(name, track, risk string) (io.ReadSeekCloser, *BinPackageInfo, error) + Exists(name, track, risk string) bool + Info(name, track, risk string) (*BinPackageInfo, error) +} + +// BinPackageInfo holds metadata about a bin package. +type BinPackageInfo struct { + Name string + Version string + Revision int + SHA384 string +} + +// Options configures a bin source. +type Options struct { + Arch string + CacheDir string +} + +type binStore struct { + options Options + cache *cache.Cache + apiURL string +} + +const ( + binsAPIBase = "https://api.snapcraft.io/v2/bins" + binsAPIStaging = "https://api.staging.snapcraft.io/v2/bins" + stagingEnvVar = "CHISEL_BINS_STAGING" +) + +var httpClient = &http.Client{ + Timeout: 30 * time.Second, +} + +var httpDo = httpClient.Do + +var bulkClient = &http.Client{ + Timeout: 5 * time.Minute, +} + +var bulkDo = bulkClient.Do + +// Open creates a new bin source with the given options. +func Open(options *Options) (Store, error) { + var err error + if options.Arch == "" { + options.Arch, err = deb.InferArch() + } else { + err = deb.ValidateArch(options.Arch) + } + if err != nil { + return nil, err + } + apiURL := binsAPIBase + if os.Getenv(stagingEnvVar) != "" { + apiURL = binsAPIStaging + } + return &binStore{ + options: *options, + cache: &cache.Cache{Dir: options.CacheDir}, + apiURL: apiURL, + }, nil +} + +// binInfoResponse represents the JSON response from the bins info endpoint. +type binInfoResponse struct { + Name string `json:"name"` + PackageID string `json:"package-id"` + ChannelMap []struct { + Channel struct { + Name string `json:"name"` + Risk string `json:"risk"` + Track string `json:"track"` + Platform struct { + Architecture string `json:"architecture"` + } `json:"platform"` + } `json:"channel"` + Revision struct { + Version string `json:"version"` + Revision int `json:"revision"` + Download struct { + URL string `json:"url"` + SHA3384 string `json:"sha3-384"` + Size int64 `json:"size"` + } `json:"download"` + Platforms []struct { + Architecture string `json:"architecture"` + } `json:"platforms"` + } `json:"revision"` + } `json:"channel-map"` +} + +func (s *binStore) fetchBinInfo(name string) (*binInfoResponse, error) { + infoURL, err := url.JoinPath(s.apiURL, "info", name) + if err != nil { + return nil, fmt.Errorf("internal error: cannot construct bins API URL: %v", err) + } + infoURL += "?fields=download,version,revision,channel-map" + + req, err := http.NewRequest("GET", infoURL, nil) + if err != nil { + return nil, fmt.Errorf("cannot create HTTP request: %v", err) + } + + resp, err := httpDo(req) + if err != nil { + return nil, fmt.Errorf("cannot talk to bins API: %v", err) + } + defer resp.Body.Close() + + switch resp.StatusCode { + case 200: + // ok + case 404: + return nil, fmt.Errorf("bin %q not found", name) + default: + return nil, fmt.Errorf("cannot fetch from bins API: %v", resp.Status) + } + + var info binInfoResponse + err = json.NewDecoder(resp.Body).Decode(&info) + if err != nil { + return nil, fmt.Errorf("cannot decode bins API response: %v", err) + } + return &info, nil +} + +// selectRevision finds the channel-map entry matching the requested track, +// risk, and architecture. +func selectRevision(info *binInfoResponse, arch, track, risk string) (downloadURL, sha3384, version string, revision int, err error) { + for _, entry := range info.ChannelMap { + if entry.Channel.Track != track || entry.Channel.Risk != risk { + continue + } + if entry.Channel.Platform.Architecture != arch { + continue + } + return entry.Revision.Download.URL, entry.Revision.Download.SHA3384, + entry.Revision.Version, entry.Revision.Revision, nil + } + return "", "", "", 0, fmt.Errorf("bin %q has no %s/%s release for architecture %q", info.Name, track, risk, arch) +} + +// allowedDownloadHosts lists the hosts from which bin downloads are permitted. +var allowedDownloadHosts = []string{ + "api.snapcraft.io", + "api.staging.snapcraft.io", + "storage.snapcraftcontent.com", +} + +// validateDownloadURL checks that the download URL is HTTPS and from an +// allowed host. +func validateDownloadURL(downloadURL string) error { + u, err := url.Parse(downloadURL) + if err != nil { + return fmt.Errorf("cannot parse bin download URL: %v", err) + } + if u.Scheme != "https" { + return fmt.Errorf("bin download URL must use HTTPS: %q", downloadURL) + } + for _, host := range allowedDownloadHosts { + if u.Host == host || strings.HasSuffix(u.Host, "."+host) { + return nil + } + } + return fmt.Errorf("bin download URL has untrusted host %q", u.Host) +} + +func (s *binStore) Info(name, track, risk string) (*BinPackageInfo, error) { + resp, err := s.fetchBinInfo(name) + if err != nil { + return nil, err + } + _, sha384, version, revision, err := selectRevision(resp, s.options.Arch, track, risk) + if err != nil { + return nil, err + } + return &BinPackageInfo{ + Name: name, + Version: version, + Revision: revision, + SHA384: sha384, + }, nil +} + +func (s *binStore) Exists(name, track, risk string) bool { + _, err := s.Info(name, track, risk) + return err == nil +} + +func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *BinPackageInfo, error) { + logf("Fetching bin %s %s/%s ...", name, track, risk) + + resp, err := s.fetchBinInfo(name) + if err != nil { + return nil, nil, err + } + downloadURL, sha384, version, revision, err := selectRevision(resp, s.options.Arch, track, risk) + if err != nil { + return nil, nil, err + } + + info := &BinPackageInfo{ + Name: name, + Version: version, + Revision: revision, + SHA384: sha384, + } + + // Check cache first. + reader, err := s.cache.Open(cache.SHA384, sha384) + if err == nil { + logf("Using cached bin %s", name) + return reader, info, nil + } else if err != cache.ErrMiss { + return nil, nil, err + } + + // Download the bin. + err = validateDownloadURL(downloadURL) + if err != nil { + return nil, nil, err + } + req, err := http.NewRequest("GET", downloadURL, nil) + if err != nil { + return nil, nil, fmt.Errorf("cannot create HTTP request: %v", err) + } + + httpResp, err := bulkDo(req) + if err != nil { + return nil, nil, fmt.Errorf("cannot download bin %q: %v", name, err) + } + defer httpResp.Body.Close() + + if httpResp.StatusCode != 200 { + return nil, nil, fmt.Errorf("cannot download bin %q: %v", name, httpResp.Status) + } + + writer := s.cache.Create(cache.SHA384, sha384) + defer writer.Close() + + _, err = io.Copy(writer, httpResp.Body) + if err == nil { + err = writer.Close() + } + if err != nil { + return nil, nil, fmt.Errorf("cannot fetch bin %q: %v", name, err) + } + + reader, err = s.cache.Open(cache.SHA384, sha384) + if err != nil { + return nil, nil, err + } + return reader, info, nil +} diff --git a/internal/store/store_test.go b/internal/store/store_test.go new file mode 100644 index 000000000..5291c3df2 --- /dev/null +++ b/internal/store/store_test.go @@ -0,0 +1,422 @@ +package store_test + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" + + "golang.org/x/crypto/sha3" + . "gopkg.in/check.v1" + + "github.com/canonical/chisel/internal/cache" + "github.com/canonical/chisel/internal/store" + "github.com/canonical/chisel/internal/testutil" +) + +type storeSuite struct { + tempDir string + cacheDir string + fakeDoFunc func(req *http.Request) (*http.Response, error) + restoreDo func() + restoreBulk func() + envRestore func() +} + +var _ = Suite(&storeSuite{}) + +func (s *storeSuite) SetUpTest(c *C) { + s.tempDir = c.MkDir() + s.cacheDir = filepath.Join(s.tempDir, "cache") + c.Assert(os.MkdirAll(s.cacheDir, 0o755), IsNil) + + s.envRestore = fakeEnv("") + s.restoreDo = store.SetHTTPDo(s.doRequest) + s.restoreBulk = store.SetBulkDo(s.doRequest) + s.fakeDoFunc = nil +} + +func (s *storeSuite) TearDownTest(c *C) { + s.restoreDo() + s.restoreBulk() + s.envRestore() +} + +func (s *storeSuite) doRequest(req *http.Request) (*http.Response, error) { + if s.fakeDoFunc != nil { + return s.fakeDoFunc(req) + } + return nil, fmt.Errorf("unexpected HTTP request: %s", req.URL.String()) +} + +func fakeEnv(staging string) func() { + oldStaging := os.Getenv(store.StagingEnvVar) + if staging != "" { + os.Setenv(store.StagingEnvVar, staging) + } else { + os.Unsetenv(store.StagingEnvVar) + } + return func() { + if oldStaging != "" { + os.Setenv(store.StagingEnvVar, oldStaging) + } else { + os.Unsetenv(store.StagingEnvVar) + } + } +} + +func sha384Hash(data []byte) string { + h := sha3.New384() + h.Write(data) + return fmt.Sprintf("%x", h.Sum(nil)) +} + +func makeBinInfoResponse(name, track, risk, arch, version string, revision int, sha384 string) *binInfoResponseJSON { + return &binInfoResponseJSON{ + Name: name, + ChannelMap: []binChannelMapJSON{ + { + Channel: binChannelJSON{ + Name: track + "/" + risk, + Risk: risk, + Track: track, + Platform: binPlatformJSON{ + Architecture: arch, + }, + }, + Revision: binRevisionJSON{ + Version: version, + Revision: revision, + Download: binDownloadJSON{ + URL: "https://storage.snapcraftcontent.com/bins/" + name + ".tar.xz", + SHA384: sha384, + Size: 1024, + }, + }, + }, + }, + } +} + +// JSON structures matching the internal binInfoResponse for test construction. +type binInfoResponseJSON struct { + Name string `json:"name"` + ChannelMap []binChannelMapJSON `json:"channel-map"` +} + +type binChannelMapJSON struct { + Channel binChannelJSON `json:"channel"` + Revision binRevisionJSON `json:"revision"` +} + +type binChannelJSON struct { + Name string `json:"name"` + Risk string `json:"risk"` + Track string `json:"track"` + Platform binPlatformJSON `json:"platform"` +} + +type binPlatformJSON struct { + Architecture string `json:"architecture"` +} + +type binRevisionJSON struct { + Version string `json:"version"` + Revision int `json:"revision"` + Download binDownloadJSON `json:"download"` +} + +type binDownloadJSON struct { + URL string `json:"url"` + SHA384 string `json:"sha3-384"` + Size int64 `json:"size"` +} + +func (s *storeSuite) TestValidateDownloadURL(c *C) { + tests := []struct { + url string + errStr string + }{ + {"https://storage.snapcraftcontent.com/bins/foo.tar.xz", ""}, + {"https://api.snapcraft.io/v2/bins/foo", ""}, + {"https://api.staging.snapcraft.io/v2/bins/foo", ""}, + {"https://sub.storage.snapcraftcontent.com/bins/foo.tar.xz", ""}, + {"http://storage.snapcraftcontent.com/bins/foo.tar.xz", "must use HTTPS"}, + {"https://evil.example.com/bins/foo.tar.xz", "untrusted host"}, + {"https://storage.snapcraftcontent.com.evil.com/bins/foo.tar.xz", "untrusted host"}, + {"://invalid-url", "cannot parse"}, + } + for _, test := range tests { + err := store.ValidateDownloadURL(test.url) + if test.errStr == "" { + c.Assert(err, IsNil) + } else { + c.Assert(err, NotNil) + c.Assert(err.Error(), testutil.Contains, test.errStr) + } + } +} + +func (s *storeSuite) TestOpenArchValidation(c *C) { + tests := []struct { + arch string + errStr string + }{ + {"amd64", ""}, + {"arm64", ""}, + {"invalid", "invalid package architecture"}, + } + for _, test := range tests { + _, err := store.Open(&store.Options{ + Arch: test.arch, + CacheDir: s.cacheDir, + }) + if test.errStr == "" { + c.Assert(err, IsNil) + } else { + c.Assert(err, NotNil) + c.Assert(err.Error(), testutil.Contains, test.errStr) + } + } +} + +func (s *storeSuite) TestInfoSuccess(c *C) { + infoResp := makeBinInfoResponse("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + infoBody, _ := json.Marshal(infoResp) + + s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + c.Assert(req.URL.Path, Equals, "/v2/bins/info/curl") + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(infoBody)), + }, nil + } + + src, err := store.Open(&store.Options{ + Arch: "amd64", + CacheDir: s.cacheDir, + }) + c.Assert(err, IsNil) + + info, err := src.Info("curl", "latest", "stable") + c.Assert(err, IsNil) + c.Assert(info.Name, Equals, "curl") + c.Assert(info.Version, Equals, "8.5.0") + c.Assert(info.Revision, Equals, 42) + c.Assert(info.SHA384, Equals, "abc123") +} + +func (s *storeSuite) TestInfoNotFound(c *C) { + s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: 404, + Body: io.NopCloser(strings.NewReader("not found")), + }, nil + } + + src, err := store.Open(&store.Options{ + Arch: "amd64", + CacheDir: s.cacheDir, + }) + c.Assert(err, IsNil) + + _, err = src.Info("nonexistent", "latest", "stable") + c.Assert(err, NotNil) + c.Assert(err.Error(), testutil.Contains, "not found") +} + +func (s *storeSuite) TestInfoNoMatchingChannel(c *C) { + infoResp := makeBinInfoResponse("curl", "latest", "stable", "arm64", "8.5.0", 42, "abc123") + infoBody, _ := json.Marshal(infoResp) + + s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(infoBody)), + }, nil + } + + src, err := store.Open(&store.Options{ + Arch: "amd64", + CacheDir: s.cacheDir, + }) + c.Assert(err, IsNil) + + // The response only has arm64, but we're asking for amd64. + _, err = src.Info("curl", "latest", "stable") + c.Assert(err, NotNil) + c.Assert(err.Error(), testutil.Contains, "has no") +} + +func (s *storeSuite) TestExists(c *C) { + infoResp := makeBinInfoResponse("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + infoBody, _ := json.Marshal(infoResp) + + s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + if strings.Contains(req.URL.Path, "/info/curl") { + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(infoBody)), + }, nil + } + return &http.Response{ + StatusCode: 404, + Body: io.NopCloser(strings.NewReader("not found")), + }, nil + } + + src, err := store.Open(&store.Options{ + Arch: "amd64", + CacheDir: s.cacheDir, + }) + c.Assert(err, IsNil) + + c.Assert(src.Exists("curl", "latest", "stable"), Equals, true) + c.Assert(src.Exists("nonexistent", "latest", "stable"), Equals, false) +} + +func (s *storeSuite) TestFetchCacheMiss(c *C) { + tarData := []byte("fake tar.xz content") + digest := sha384Hash(tarData) + + infoResp := makeBinInfoResponse("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) + infoBody, _ := json.Marshal(infoResp) + + callCount := 0 + s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + callCount++ + if strings.Contains(req.URL.Path, "/info/") { + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(infoBody)), + }, nil + } + // Download URL + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(tarData)), + }, nil + } + + src, err := store.Open(&store.Options{ + Arch: "amd64", + CacheDir: s.cacheDir, + }) + c.Assert(err, IsNil) + + reader, info, err := src.Fetch("curl", "latest", "stable") + c.Assert(err, IsNil) + defer reader.Close() + + c.Assert(info.Name, Equals, "curl") + c.Assert(info.Version, Equals, "8.5.0") + c.Assert(info.SHA384, Equals, digest) + + data, err := io.ReadAll(reader) + c.Assert(err, IsNil) + c.Assert(data, DeepEquals, tarData) + c.Assert(callCount, Equals, 2) + + // Verify it's in the cache. + cc := cache.Cache{Dir: s.cacheDir} + cached, err := cc.Read(cache.SHA384, digest) + c.Assert(err, IsNil) + c.Assert(cached, DeepEquals, tarData) +} + +func (s *storeSuite) TestFetchCacheHit(c *C) { + tarData := []byte("fake tar.xz content") + digest := sha384Hash(tarData) + + // Pre-populate the cache. + cc := cache.Cache{Dir: s.cacheDir} + err := cc.Write(cache.SHA384, digest, tarData) + c.Assert(err, IsNil) + + infoResp := makeBinInfoResponse("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) + infoBody, _ := json.Marshal(infoResp) + + infoCallCount := 0 + s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + if strings.Contains(req.URL.Path, "/info/") { + infoCallCount++ + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(infoBody)), + }, nil + } + return nil, fmt.Errorf("download should not be called for cache hit") + } + + src, err := store.Open(&store.Options{ + Arch: "amd64", + CacheDir: s.cacheDir, + }) + c.Assert(err, IsNil) + + reader, info, err := src.Fetch("curl", "latest", "stable") + c.Assert(err, IsNil) + defer reader.Close() + + c.Assert(info.Name, Equals, "curl") + c.Assert(info.SHA384, Equals, digest) + c.Assert(infoCallCount, Equals, 1) + + data, err := io.ReadAll(reader) + c.Assert(err, IsNil) + c.Assert(data, DeepEquals, tarData) +} + +func (s *storeSuite) TestFetchInvalidDownloadURL(c *C) { + infoResp := makeBinInfoResponse("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + // Override the download URL to an invalid one. + infoResp.ChannelMap[0].Revision.Download.URL = "http://evil.example.com/bins/curl.tar.xz" + infoBody, _ := json.Marshal(infoResp) + + s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(infoBody)), + }, nil + } + + src, err := store.Open(&store.Options{ + Arch: "amd64", + CacheDir: s.cacheDir, + }) + c.Assert(err, IsNil) + + _, _, err = src.Fetch("curl", "latest", "stable") + c.Assert(err, NotNil) + c.Assert(err.Error(), testutil.Contains, "must use HTTPS") +} + +func (s *storeSuite) TestStagingEnvVar(c *C) { + s.envRestore() + s.envRestore = fakeEnv("1") + + src, err := store.Open(&store.Options{ + Arch: "amd64", + CacheDir: s.cacheDir, + }) + c.Assert(err, IsNil) + + infoResp := makeBinInfoResponse("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + infoBody, _ := json.Marshal(infoResp) + + s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + // Verify staging URL is used. + c.Assert(req.URL.Host, Equals, "api.staging.snapcraft.io") + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(infoBody)), + }, nil + } + + _, err = src.Info("curl", "latest", "stable") + c.Assert(err, IsNil) +} diff --git a/internal/store/suite_test.go b/internal/store/suite_test.go new file mode 100644 index 000000000..552ab1cc8 --- /dev/null +++ b/internal/store/suite_test.go @@ -0,0 +1,13 @@ +package store_test + +import ( + "testing" + + . "gopkg.in/check.v1" +) + +func Test(t *testing.T) { TestingT(t) } + +type S struct{} + +var _ = Suite(&S{}) From 28b92444728b175a8b80b73d7dd3b70ba68ee689 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 19 Jun 2026 16:30:59 +0200 Subject: [PATCH 02/20] fix: adjust naming --- cmd/chisel/cmd_cut.go | 1 + internal/store/store.go | 53 ++++++++++++++++++++++++----------------- 2 files changed, 32 insertions(+), 22 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index a37967312..db56124a0 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -127,6 +127,7 @@ func (cmd *cmdCut) Execute(args []string) error { openStore, err := store.Open(&store.Options{ Arch: cmd.Arch, CacheDir: cache.DefaultDir("chisel"), + Kind: storeInfo.Kind, }) if err != nil { return err diff --git a/internal/store/store.go b/internal/store/store.go index c169ed3c2..25dd68996 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -14,27 +14,31 @@ import ( "github.com/canonical/chisel/internal/deb" ) -// Store provides access to bin packages from the Snapcraft store API. +// Store provides access to packages from the Snapcraft store API. type Store interface { - Fetch(name, track, risk string) (io.ReadSeekCloser, *BinPackageInfo, error) + Fetch(name, track, risk string) (io.ReadSeekCloser, *StorePackageInfo, error) Exists(name, track, risk string) bool - Info(name, track, risk string) (*BinPackageInfo, error) + Info(name, track, risk string) (*StorePackageInfo, error) } -// BinPackageInfo holds metadata about a bin package. -type BinPackageInfo struct { +// StorePackageInfo holds metadata about a package. +type StorePackageInfo struct { Name string Version string Revision int SHA384 string } -// Options configures a bin source. type Options struct { Arch string CacheDir string + Kind string } +type storeKind string + +const storeKindBin storeKind = "bin" + type binStore struct { options Options cache *cache.Cache @@ -42,9 +46,9 @@ type binStore struct { } const ( - binsAPIBase = "https://api.snapcraft.io/v2/bins" - binsAPIStaging = "https://api.staging.snapcraft.io/v2/bins" - stagingEnvVar = "CHISEL_BINS_STAGING" + binAPIBase = "https://api.snapcraft.io/v2/bins" + binAPIStaging = "https://api.staging.snapcraft.io/v2/bins" + stagingEnvVar = "CHISEL_BIN_STAGING" ) var httpClient = &http.Client{ @@ -59,7 +63,6 @@ var bulkClient = &http.Client{ var bulkDo = bulkClient.Do -// Open creates a new bin source with the given options. func Open(options *Options) (Store, error) { var err error if options.Arch == "" { @@ -70,15 +73,21 @@ func Open(options *Options) (Store, error) { if err != nil { return nil, err } - apiURL := binsAPIBase - if os.Getenv(stagingEnvVar) != "" { - apiURL = binsAPIStaging + + switch storeKind(options.Kind) { + case storeKindBin: + apiURL := binAPIBase + if os.Getenv(stagingEnvVar) != "" { + apiURL = binAPIStaging + } + return &binStore{ + options: *options, + cache: &cache.Cache{Dir: options.CacheDir}, + apiURL: apiURL, + }, nil + default: + return nil, fmt.Errorf("unsupported store kind %q", options.Kind) } - return &binStore{ - options: *options, - cache: &cache.Cache{Dir: options.CacheDir}, - apiURL: apiURL, - }, nil } // binInfoResponse represents the JSON response from the bins info endpoint. @@ -185,7 +194,7 @@ func validateDownloadURL(downloadURL string) error { return fmt.Errorf("bin download URL has untrusted host %q", u.Host) } -func (s *binStore) Info(name, track, risk string) (*BinPackageInfo, error) { +func (s *binStore) Info(name, track, risk string) (*StorePackageInfo, error) { resp, err := s.fetchBinInfo(name) if err != nil { return nil, err @@ -194,7 +203,7 @@ func (s *binStore) Info(name, track, risk string) (*BinPackageInfo, error) { if err != nil { return nil, err } - return &BinPackageInfo{ + return &StorePackageInfo{ Name: name, Version: version, Revision: revision, @@ -207,7 +216,7 @@ func (s *binStore) Exists(name, track, risk string) bool { return err == nil } -func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *BinPackageInfo, error) { +func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *StorePackageInfo, error) { logf("Fetching bin %s %s/%s ...", name, track, risk) resp, err := s.fetchBinInfo(name) @@ -219,7 +228,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *BinPacka return nil, nil, err } - info := &BinPackageInfo{ + info := &StorePackageInfo{ Name: name, Version: version, Revision: revision, From c4b53fddb5c67180e3d787a5c1724e4a015a672c Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 19 Jun 2026 16:45:57 +0200 Subject: [PATCH 03/20] style: linting errors --- cmd/chisel/cmd_cut.go | 2 +- internal/slicer/slicer.go | 2 +- internal/store/store.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index db56124a0..6e4db2255 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -8,10 +8,10 @@ import ( "github.com/jessevdk/go-flags" "github.com/canonical/chisel/internal/archive" - "github.com/canonical/chisel/internal/store" "github.com/canonical/chisel/internal/cache" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" + "github.com/canonical/chisel/internal/store" ) var shortCutHelp = "Cut a tree with selected slices" diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 426012de1..71af485b7 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -16,11 +16,11 @@ import ( "github.com/klauspost/compress/zstd" "github.com/canonical/chisel/internal/archive" - "github.com/canonical/chisel/internal/store" "github.com/canonical/chisel/internal/fsutil" "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/scripts" "github.com/canonical/chisel/internal/setup" + "github.com/canonical/chisel/internal/store" "github.com/canonical/chisel/internal/tarball" ) diff --git a/internal/store/store.go b/internal/store/store.go index 25dd68996..03ff28eef 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -32,7 +32,7 @@ type StorePackageInfo struct { type Options struct { Arch string CacheDir string - Kind string + Kind string } type storeKind string @@ -48,7 +48,7 @@ type binStore struct { const ( binAPIBase = "https://api.snapcraft.io/v2/bins" binAPIStaging = "https://api.staging.snapcraft.io/v2/bins" - stagingEnvVar = "CHISEL_BIN_STAGING" + stagingEnvVar = "CHISEL_BIN_STAGING" ) var httpClient = &http.Client{ From ce800d70fd02b80e0f56f2dc89e2e949ead944cd Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 22 Jun 2026 14:18:44 +0200 Subject: [PATCH 04/20] feat: refining --- internal/store/export_test.go | 2 +- internal/store/store.go | 27 ++- internal/store/store_test.go | 323 ++++++++++++++++++++-------------- 3 files changed, 205 insertions(+), 147 deletions(-) diff --git a/internal/store/export_test.go b/internal/store/export_test.go index b4c64aadb..f69322be8 100644 --- a/internal/store/export_test.go +++ b/internal/store/export_test.go @@ -4,7 +4,7 @@ import "net/http" var ( ValidateDownloadURL = validateDownloadURL - StagingEnvVar = stagingEnvVar + BinStagingEnvVar = binStagingEnvVar ) func SetHTTPDo(fn func(req *http.Request) (*http.Response, error)) (restore func()) { diff --git a/internal/store/store.go b/internal/store/store.go index 03ff28eef..ec39cd45e 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -7,6 +7,7 @@ import ( "net/http" "net/url" "os" + "regexp" "strings" "time" @@ -46,9 +47,9 @@ type binStore struct { } const ( - binAPIBase = "https://api.snapcraft.io/v2/bins" - binAPIStaging = "https://api.staging.snapcraft.io/v2/bins" - stagingEnvVar = "CHISEL_BIN_STAGING" + binAPIBase = "https://api.snapcraft.io/v2/bins" + binAPIStaging = "https://api.staging.snapcraft.io/v2/bins" + binStagingEnvVar = "CHISEL_BIN_STAGING" ) var httpClient = &http.Client{ @@ -77,7 +78,7 @@ func Open(options *Options) (Store, error) { switch storeKind(options.Kind) { case storeKindBin: apiURL := binAPIBase - if os.Getenv(stagingEnvVar) != "" { + if os.Getenv(binStagingEnvVar) != "" { apiURL = binAPIStaging } return &binStore{ @@ -90,7 +91,7 @@ func Open(options *Options) (Store, error) { } } -// binInfoResponse represents the JSON response from the bins info endpoint. +// binInfoResponse represents the JSON response from the bin store info endpoint. type binInfoResponse struct { Name string `json:"name"` PackageID string `json:"package-id"` @@ -119,9 +120,12 @@ type binInfoResponse struct { } func (s *binStore) fetchBinInfo(name string) (*binInfoResponse, error) { + if !nameExp.MatchString(name) { + return nil, fmt.Errorf("invalid package name %q", name) + } infoURL, err := url.JoinPath(s.apiURL, "info", name) if err != nil { - return nil, fmt.Errorf("internal error: cannot construct bins API URL: %v", err) + return nil, fmt.Errorf("internal error: cannot construct bin store URL: %v", err) } infoURL += "?fields=download,version,revision,channel-map" @@ -132,7 +136,7 @@ func (s *binStore) fetchBinInfo(name string) (*binInfoResponse, error) { resp, err := httpDo(req) if err != nil { - return nil, fmt.Errorf("cannot talk to bins API: %v", err) + return nil, fmt.Errorf("cannot talk to bin store: %v", err) } defer resp.Body.Close() @@ -142,13 +146,13 @@ func (s *binStore) fetchBinInfo(name string) (*binInfoResponse, error) { case 404: return nil, fmt.Errorf("bin %q not found", name) default: - return nil, fmt.Errorf("cannot fetch from bins API: %v", resp.Status) + return nil, fmt.Errorf("cannot fetch from bin store: %v", resp.Status) } var info binInfoResponse err = json.NewDecoder(resp.Body).Decode(&info) if err != nil { - return nil, fmt.Errorf("cannot decode bins API response: %v", err) + return nil, fmt.Errorf("cannot decode bin store response: %v", err) } return &info, nil } @@ -169,6 +173,11 @@ func selectRevision(info *binInfoResponse, arch, track, risk string) (downloadUR return "", "", "", 0, fmt.Errorf("bin %q has no %s/%s release for architecture %q", info.Name, track, risk, arch) } +// nameExp matches a valid package name. It deliberately forbids "/" and any +// leading "." so that a name cannot be used to traverse or otherwise alter the +// store API URL path when interpolated into it. +var nameExp = regexp.MustCompile(`^[a-z0-9][a-z0-9+.-]*$`) + // allowedDownloadHosts lists the hosts from which bin downloads are permitted. var allowedDownloadHosts = []string{ "api.snapcraft.io", diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 5291c3df2..e1525c0bd 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -2,7 +2,6 @@ package store_test import ( "bytes" - "encoding/json" "fmt" "io" "net/http" @@ -15,7 +14,6 @@ import ( "github.com/canonical/chisel/internal/cache" "github.com/canonical/chisel/internal/store" - "github.com/canonical/chisel/internal/testutil" ) type storeSuite struct { @@ -54,17 +52,17 @@ func (s *storeSuite) doRequest(req *http.Request) (*http.Response, error) { } func fakeEnv(staging string) func() { - oldStaging := os.Getenv(store.StagingEnvVar) + oldStaging := os.Getenv(store.BinStagingEnvVar) if staging != "" { - os.Setenv(store.StagingEnvVar, staging) + os.Setenv(store.BinStagingEnvVar, staging) } else { - os.Unsetenv(store.StagingEnvVar) + os.Unsetenv(store.BinStagingEnvVar) } return func() { if oldStaging != "" { - os.Setenv(store.StagingEnvVar, oldStaging) + os.Setenv(store.BinStagingEnvVar, oldStaging) } else { - os.Unsetenv(store.StagingEnvVar) + os.Unsetenv(store.BinStagingEnvVar) } } } @@ -75,165 +73,181 @@ func sha384Hash(data []byte) string { return fmt.Sprintf("%x", h.Sum(nil)) } -func makeBinInfoResponse(name, track, risk, arch, version string, revision int, sha384 string) *binInfoResponseJSON { - return &binInfoResponseJSON{ - Name: name, - ChannelMap: []binChannelMapJSON{ - { - Channel: binChannelJSON{ - Name: track + "/" + risk, - Risk: risk, - Track: track, - Platform: binPlatformJSON{ - Architecture: arch, - }, - }, - Revision: binRevisionJSON{ - Version: version, - Revision: revision, - Download: binDownloadJSON{ - URL: "https://storage.snapcraftcontent.com/bins/" + name + ".tar.xz", - SHA384: sha384, - Size: 1024, - }, - }, - }, - }, - } -} - -// JSON structures matching the internal binInfoResponse for test construction. -type binInfoResponseJSON struct { - Name string `json:"name"` - ChannelMap []binChannelMapJSON `json:"channel-map"` -} - -type binChannelMapJSON struct { - Channel binChannelJSON `json:"channel"` - Revision binRevisionJSON `json:"revision"` -} - -type binChannelJSON struct { - Name string `json:"name"` - Risk string `json:"risk"` - Track string `json:"track"` - Platform binPlatformJSON `json:"platform"` -} - -type binPlatformJSON struct { - Architecture string `json:"architecture"` +func makeBinInfoBody(name, track, risk, arch, version string, revision int, sha384 string) []byte { + downloadURL := "https://storage.snapcraftcontent.com/bins/" + name + ".tar.xz" + return makeBinInfoBodyWithURL(name, track, risk, arch, version, revision, sha384, downloadURL) } -type binRevisionJSON struct { - Version string `json:"version"` - Revision int `json:"revision"` - Download binDownloadJSON `json:"download"` -} - -type binDownloadJSON struct { - URL string `json:"url"` - SHA384 string `json:"sha3-384"` - Size int64 `json:"size"` +func makeBinInfoBodyWithURL(name, track, risk, arch, version string, revision int, sha384, downloadURL string) []byte { + return []byte(fmt.Sprintf(`{ + "name": %q, + "channel-map": [ + { + "channel": { + "name": %q, + "risk": %q, + "track": %q, + "platform": {"architecture": %q} + }, + "revision": { + "version": %q, + "revision": %d, + "download": { + "url": %q, + "sha3-384": %q, + "size": 1024 + } + } + } + ] + }`, name, track+"/"+risk, risk, track, arch, version, revision, downloadURL, sha384)) } func (s *storeSuite) TestValidateDownloadURL(c *C) { tests := []struct { - url string - errStr string + url string + error string }{ {"https://storage.snapcraftcontent.com/bins/foo.tar.xz", ""}, {"https://api.snapcraft.io/v2/bins/foo", ""}, {"https://api.staging.snapcraft.io/v2/bins/foo", ""}, {"https://sub.storage.snapcraftcontent.com/bins/foo.tar.xz", ""}, - {"http://storage.snapcraftcontent.com/bins/foo.tar.xz", "must use HTTPS"}, - {"https://evil.example.com/bins/foo.tar.xz", "untrusted host"}, - {"https://storage.snapcraftcontent.com.evil.com/bins/foo.tar.xz", "untrusted host"}, - {"://invalid-url", "cannot parse"}, + { + "http://storage.snapcraftcontent.com/bins/foo.tar.xz", + `bin download URL must use HTTPS: "http://storage.snapcraftcontent.com/bins/foo.tar.xz"`, + }, + { + "https://evil.example.com/bins/foo.tar.xz", + `bin download URL has untrusted host "evil.example.com"`, + }, + { + "https://storage.snapcraftcontent.com.evil.com/bins/foo.tar.xz", + `bin download URL has untrusted host "storage.snapcraftcontent.com.evil.com"`, + }, + {"://invalid-url", `cannot parse bin download URL: .*`}, } for _, test := range tests { err := store.ValidateDownloadURL(test.url) - if test.errStr == "" { + if test.error == "" { c.Assert(err, IsNil) } else { - c.Assert(err, NotNil) - c.Assert(err.Error(), testutil.Contains, test.errStr) + c.Assert(err, ErrorMatches, test.error) } } } func (s *storeSuite) TestOpenArchValidation(c *C) { tests := []struct { - arch string - errStr string + arch string + error string }{ {"amd64", ""}, {"arm64", ""}, - {"invalid", "invalid package architecture"}, + {"invalid", "invalid package architecture: invalid"}, } for _, test := range tests { _, err := store.Open(&store.Options{ Arch: test.arch, CacheDir: s.cacheDir, + Kind: "bin", }) - if test.errStr == "" { + if test.error == "" { c.Assert(err, IsNil) } else { - c.Assert(err, NotNil) - c.Assert(err.Error(), testutil.Contains, test.errStr) + c.Assert(err, ErrorMatches, test.error) } } } -func (s *storeSuite) TestInfoSuccess(c *C) { - infoResp := makeBinInfoResponse("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") - infoBody, _ := json.Marshal(infoResp) - - s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { - c.Assert(req.URL.Path, Equals, "/v2/bins/info/curl") - return &http.Response{ - StatusCode: 200, - Body: io.NopCloser(bytes.NewReader(infoBody)), - }, nil - } - - src, err := store.Open(&store.Options{ - Arch: "amd64", - CacheDir: s.cacheDir, - }) - c.Assert(err, IsNil) - - info, err := src.Info("curl", "latest", "stable") - c.Assert(err, IsNil) - c.Assert(info.Name, Equals, "curl") - c.Assert(info.Version, Equals, "8.5.0") - c.Assert(info.Revision, Equals, 42) - c.Assert(info.SHA384, Equals, "abc123") +type infoTest struct { + summary string + status int + statusText string + body string + info *store.StorePackageInfo + error string } -func (s *storeSuite) TestInfoNotFound(c *C) { - s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { - return &http.Response{ - StatusCode: 404, - Body: io.NopCloser(strings.NewReader("not found")), - }, nil - } +var infoTests = []infoTest{{ + summary: "Successful info", + status: 200, + body: string(makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123")), + info: &store.StorePackageInfo{Name: "curl", Version: "8.5.0", Revision: 42, SHA384: "abc123"}, +}, { + summary: "Package not found", + status: 404, + body: "not found", + error: `bin "curl" not found`, +}, { + summary: "No release for the requested architecture", + status: 200, + body: string(makeBinInfoBody("curl", "latest", "stable", "arm64", "8.5.0", 42, "abc123")), + error: `bin "curl" has no latest/stable release for architecture "amd64"`, +}, { + summary: "Server error", + status: 500, + statusText: "500 Internal Server Error", + body: "boom", + error: "cannot fetch from bin store: 500 Internal Server Error", +}, { + summary: "Malformed response body", + status: 200, + body: "not json", + error: "cannot decode bin store response: .*", +}, { + summary: "Selects the entry matching the requested architecture", + status: 200, + body: `{ + "name": "curl", + "channel-map": [ + { + "channel": {"name": "latest/stable", "risk": "stable", "track": "latest", "platform": {"architecture": "arm64"}}, + "revision": {"version": "8.0.0", "revision": 1, "download": {"url": "https://storage.snapcraftcontent.com/bins/curl.tar.xz", "sha3-384": "arm64hash", "size": 1024}} + }, + { + "channel": {"name": "latest/stable", "risk": "stable", "track": "latest", "platform": {"architecture": "amd64"}}, + "revision": {"version": "8.5.0", "revision": 42, "download": {"url": "https://storage.snapcraftcontent.com/bins/curl.tar.xz", "sha3-384": "amd64hash", "size": 1024}} + } + ] + }`, + info: &store.StorePackageInfo{Name: "curl", Version: "8.5.0", Revision: 42, SHA384: "amd64hash"}, +}} + +func (s *storeSuite) TestInfo(c *C) { + for _, test := range infoTests { + c.Logf("Summary: %s", test.summary) + + s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: test.status, + Status: test.statusText, + Body: io.NopCloser(strings.NewReader(test.body)), + }, nil + } - src, err := store.Open(&store.Options{ - Arch: "amd64", - CacheDir: s.cacheDir, - }) - c.Assert(err, IsNil) + src, err := store.Open(&store.Options{ + Arch: "amd64", + CacheDir: s.cacheDir, + Kind: "bin", + }) + c.Assert(err, IsNil) - _, err = src.Info("nonexistent", "latest", "stable") - c.Assert(err, NotNil) - c.Assert(err.Error(), testutil.Contains, "not found") + info, err := src.Info("curl", "latest", "stable") + if test.error != "" { + c.Assert(err, ErrorMatches, test.error) + continue + } + c.Assert(err, IsNil) + c.Assert(info, DeepEquals, test.info) + } } -func (s *storeSuite) TestInfoNoMatchingChannel(c *C) { - infoResp := makeBinInfoResponse("curl", "latest", "stable", "arm64", "8.5.0", 42, "abc123") - infoBody, _ := json.Marshal(infoResp) +func (s *storeSuite) TestInfoRequest(c *C) { + infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + c.Assert(req.URL.Path, Equals, "/v2/bins/info/curl") + c.Assert(req.URL.Query().Get("fields"), Equals, "download,version,revision,channel-map") return &http.Response{ StatusCode: 200, Body: io.NopCloser(bytes.NewReader(infoBody)), @@ -243,18 +257,16 @@ func (s *storeSuite) TestInfoNoMatchingChannel(c *C) { src, err := store.Open(&store.Options{ Arch: "amd64", CacheDir: s.cacheDir, + Kind: "bin", }) c.Assert(err, IsNil) - // The response only has arm64, but we're asking for amd64. _, err = src.Info("curl", "latest", "stable") - c.Assert(err, NotNil) - c.Assert(err.Error(), testutil.Contains, "has no") + c.Assert(err, IsNil) } func (s *storeSuite) TestExists(c *C) { - infoResp := makeBinInfoResponse("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") - infoBody, _ := json.Marshal(infoResp) + infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { if strings.Contains(req.URL.Path, "/info/curl") { @@ -272,6 +284,7 @@ func (s *storeSuite) TestExists(c *C) { src, err := store.Open(&store.Options{ Arch: "amd64", CacheDir: s.cacheDir, + Kind: "bin", }) c.Assert(err, IsNil) @@ -283,8 +296,7 @@ func (s *storeSuite) TestFetchCacheMiss(c *C) { tarData := []byte("fake tar.xz content") digest := sha384Hash(tarData) - infoResp := makeBinInfoResponse("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) - infoBody, _ := json.Marshal(infoResp) + infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) callCount := 0 s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { @@ -305,6 +317,7 @@ func (s *storeSuite) TestFetchCacheMiss(c *C) { src, err := store.Open(&store.Options{ Arch: "amd64", CacheDir: s.cacheDir, + Kind: "bin", }) c.Assert(err, IsNil) @@ -337,8 +350,7 @@ func (s *storeSuite) TestFetchCacheHit(c *C) { err := cc.Write(cache.SHA384, digest, tarData) c.Assert(err, IsNil) - infoResp := makeBinInfoResponse("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) - infoBody, _ := json.Marshal(infoResp) + infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) infoCallCount := 0 s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { @@ -355,6 +367,7 @@ func (s *storeSuite) TestFetchCacheHit(c *C) { src, err := store.Open(&store.Options{ Arch: "amd64", CacheDir: s.cacheDir, + Kind: "bin", }) c.Assert(err, IsNil) @@ -372,10 +385,9 @@ func (s *storeSuite) TestFetchCacheHit(c *C) { } func (s *storeSuite) TestFetchInvalidDownloadURL(c *C) { - infoResp := makeBinInfoResponse("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") // Override the download URL to an invalid one. - infoResp.ChannelMap[0].Revision.Download.URL = "http://evil.example.com/bins/curl.tar.xz" - infoBody, _ := json.Marshal(infoResp) + infoBody := makeBinInfoBodyWithURL("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123", + "http://evil.example.com/bins/curl.tar.xz") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { return &http.Response{ @@ -387,12 +399,12 @@ func (s *storeSuite) TestFetchInvalidDownloadURL(c *C) { src, err := store.Open(&store.Options{ Arch: "amd64", CacheDir: s.cacheDir, + Kind: "bin", }) c.Assert(err, IsNil) _, _, err = src.Fetch("curl", "latest", "stable") - c.Assert(err, NotNil) - c.Assert(err.Error(), testutil.Contains, "must use HTTPS") + c.Assert(err, ErrorMatches, `bin download URL must use HTTPS: "http://evil.example.com/bins/curl.tar.xz"`) } func (s *storeSuite) TestStagingEnvVar(c *C) { @@ -402,11 +414,11 @@ func (s *storeSuite) TestStagingEnvVar(c *C) { src, err := store.Open(&store.Options{ Arch: "amd64", CacheDir: s.cacheDir, + Kind: "bin", }) c.Assert(err, IsNil) - infoResp := makeBinInfoResponse("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") - infoBody, _ := json.Marshal(infoResp) + infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { // Verify staging URL is used. @@ -420,3 +432,40 @@ func (s *storeSuite) TestStagingEnvVar(c *C) { _, err = src.Info("curl", "latest", "stable") c.Assert(err, IsNil) } + +func (s *storeSuite) TestOpenUnsupportedKind(c *C) { + _, err := store.Open(&store.Options{ + Arch: "amd64", + CacheDir: s.cacheDir, + Kind: "snap", + }) + c.Assert(err, ErrorMatches, `unsupported store kind "snap"`) +} + +func (s *storeSuite) TestFetchDownloadError(c *C) { + infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + + s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + if strings.Contains(req.URL.Path, "/info/") { + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(infoBody)), + }, nil + } + return &http.Response{ + StatusCode: 500, + Status: "500 Internal Server Error", + Body: io.NopCloser(strings.NewReader("boom")), + }, nil + } + + src, err := store.Open(&store.Options{ + Arch: "amd64", + CacheDir: s.cacheDir, + Kind: "bin", + }) + c.Assert(err, IsNil) + + _, _, err = src.Fetch("curl", "latest", "stable") + c.Assert(err, ErrorMatches, `cannot download bin "curl": 500 Internal Server Error`) +} From 217330d9507bf9bec3d48bc36331a48aa8181e89 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 22 Jun 2026 14:31:22 +0200 Subject: [PATCH 05/20] fix: open all stores --- cmd/chisel/cmd_cut.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 6e4db2255..1ba1f2033 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -133,7 +133,6 @@ func (cmd *cmdCut) Execute(args []string) error { return err } stores[storeInfo.Name] = openStore - break } err = slicer.Run(&slicer.RunOptions{ From bc63f3102e556bc7098cddcd5a28eba0bcfa0d62 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 22 Jun 2026 14:39:54 +0200 Subject: [PATCH 06/20] fix: reapply --- internal/store/store.go | 104 ++++++++++++++++++++++------------------ 1 file changed, 57 insertions(+), 47 deletions(-) diff --git a/internal/store/store.go b/internal/store/store.go index ec39cd45e..b842fb086 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -93,43 +93,53 @@ func Open(options *Options) (Store, error) { // binInfoResponse represents the JSON response from the bin store info endpoint. type binInfoResponse struct { - Name string `json:"name"` - PackageID string `json:"package-id"` - ChannelMap []struct { - Channel struct { - Name string `json:"name"` - Risk string `json:"risk"` - Track string `json:"track"` - Platform struct { - Architecture string `json:"architecture"` - } `json:"platform"` - } `json:"channel"` - Revision struct { - Version string `json:"version"` - Revision int `json:"revision"` - Download struct { - URL string `json:"url"` - SHA3384 string `json:"sha3-384"` - Size int64 `json:"size"` - } `json:"download"` - Platforms []struct { - Architecture string `json:"architecture"` - } `json:"platforms"` - } `json:"revision"` - } `json:"channel-map"` + Name string `json:"name"` + PackageID string `json:"package-id"` + ChannelMap []binChannelMap `json:"channel-map"` +} + +type binChannelMap struct { + Channel binChannel `json:"channel"` + Revision binRevision `json:"revision"` +} + +type binChannel struct { + Name string `json:"name"` + Risk string `json:"risk"` + Track string `json:"track"` + Platform binPlatform `json:"platform"` +} + +type binPlatform struct { + Architecture string `json:"architecture"` +} + +type binRevision struct { + Version string `json:"version"` + Revision int `json:"revision"` + Download binDownload `json:"download"` +} + +type binDownload struct { + URL string `json:"url"` + SHA3384 string `json:"sha3-384"` + Size int64 `json:"size"` } func (s *binStore) fetchBinInfo(name string) (*binInfoResponse, error) { if !nameExp.MatchString(name) { return nil, fmt.Errorf("invalid package name %q", name) } - infoURL, err := url.JoinPath(s.apiURL, "info", name) + u, err := url.Parse(s.apiURL) if err != nil { - return nil, fmt.Errorf("internal error: cannot construct bin store URL: %v", err) + return nil, fmt.Errorf("internal error: cannot parse bin store URL: %v", err) } - infoURL += "?fields=download,version,revision,channel-map" + u = u.JoinPath("info", name) + u.RawQuery = url.Values{ + "fields": {"download,version,revision,channel-map"}, + }.Encode() - req, err := http.NewRequest("GET", infoURL, nil) + req, err := http.NewRequest("GET", u.String(), nil) if err != nil { return nil, fmt.Errorf("cannot create HTTP request: %v", err) } @@ -158,19 +168,19 @@ func (s *binStore) fetchBinInfo(name string) (*binInfoResponse, error) { } // selectRevision finds the channel-map entry matching the requested track, -// risk, and architecture. -func selectRevision(info *binInfoResponse, arch, track, risk string) (downloadURL, sha3384, version string, revision int, err error) { - for _, entry := range info.ChannelMap { +// risk, and architecture, returning its revision. +func selectRevision(info *binInfoResponse, arch, track, risk string) (*binRevision, error) { + for i := range info.ChannelMap { + entry := &info.ChannelMap[i] if entry.Channel.Track != track || entry.Channel.Risk != risk { continue } if entry.Channel.Platform.Architecture != arch { continue } - return entry.Revision.Download.URL, entry.Revision.Download.SHA3384, - entry.Revision.Version, entry.Revision.Revision, nil + return &entry.Revision, nil } - return "", "", "", 0, fmt.Errorf("bin %q has no %s/%s release for architecture %q", info.Name, track, risk, arch) + return nil, fmt.Errorf("bin %q has no %s/%s release for architecture %q", info.Name, track, risk, arch) } // nameExp matches a valid package name. It deliberately forbids "/" and any @@ -208,15 +218,15 @@ func (s *binStore) Info(name, track, risk string) (*StorePackageInfo, error) { if err != nil { return nil, err } - _, sha384, version, revision, err := selectRevision(resp, s.options.Arch, track, risk) + rev, err := selectRevision(resp, s.options.Arch, track, risk) if err != nil { return nil, err } return &StorePackageInfo{ Name: name, - Version: version, - Revision: revision, - SHA384: sha384, + Version: rev.Version, + Revision: rev.Revision, + SHA384: rev.Download.SHA3384, }, nil } @@ -232,20 +242,20 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *StorePac if err != nil { return nil, nil, err } - downloadURL, sha384, version, revision, err := selectRevision(resp, s.options.Arch, track, risk) + rev, err := selectRevision(resp, s.options.Arch, track, risk) if err != nil { return nil, nil, err } info := &StorePackageInfo{ Name: name, - Version: version, - Revision: revision, - SHA384: sha384, + Version: rev.Version, + Revision: rev.Revision, + SHA384: rev.Download.SHA3384, } // Check cache first. - reader, err := s.cache.Open(cache.SHA384, sha384) + reader, err := s.cache.Open(cache.SHA384, rev.Download.SHA3384) if err == nil { logf("Using cached bin %s", name) return reader, info, nil @@ -254,11 +264,11 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *StorePac } // Download the bin. - err = validateDownloadURL(downloadURL) + err = validateDownloadURL(rev.Download.URL) if err != nil { return nil, nil, err } - req, err := http.NewRequest("GET", downloadURL, nil) + req, err := http.NewRequest("GET", rev.Download.URL, nil) if err != nil { return nil, nil, fmt.Errorf("cannot create HTTP request: %v", err) } @@ -273,7 +283,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *StorePac return nil, nil, fmt.Errorf("cannot download bin %q: %v", name, httpResp.Status) } - writer := s.cache.Create(cache.SHA384, sha384) + writer := s.cache.Create(cache.SHA384, rev.Download.SHA3384) defer writer.Close() _, err = io.Copy(writer, httpResp.Body) @@ -284,7 +294,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *StorePac return nil, nil, fmt.Errorf("cannot fetch bin %q: %v", name, err) } - reader, err = s.cache.Open(cache.SHA384, sha384) + reader, err = s.cache.Open(cache.SHA384, rev.Download.SHA3384) if err != nil { return nil, nil, err } From ff51f2f3ae670eb57e11262aa00299fcd361259a Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 22 Jun 2026 17:38:28 +0200 Subject: [PATCH 07/20] feat: wire to Run --- internal/archive/archive.go | 10 ++++---- internal/slicer/slicer.go | 26 ++++++++++++-------- internal/slicer/slicer_test.go | 24 +++++++++++++++++++ internal/store/store.go | 31 ++++++++++++------------ internal/testutil/archive.go | 9 ------- internal/testutil/package.go | 17 ++++++++++++++ internal/testutil/store.go | 43 ++++++++++++++++++++++++++++++++++ 7 files changed, 122 insertions(+), 38 deletions(-) create mode 100644 internal/testutil/package.go create mode 100644 internal/testutil/store.go diff --git a/internal/archive/archive.go b/internal/archive/archive.go index 7c90a41dd..c88a30bda 100644 --- a/internal/archive/archive.go +++ b/internal/archive/archive.go @@ -26,10 +26,12 @@ type Archive interface { } type PackageInfo struct { - Name string - Version string - Arch string - SHA256 string + Name string + Version string + Arch string + SHA256 string + Revision int + SHA384 string } type Options struct { diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 71af485b7..365d3d66e 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -24,7 +24,7 @@ import ( "github.com/canonical/chisel/internal/tarball" ) -const manifestMode fs.FileMode = 0644 +const manifestMode fs.FileMode = 0o644 type RunOptions struct { Selection *setup.Selection @@ -101,7 +101,7 @@ func Run(options *RunOptions) error { targetDir = filepath.Join(dir, targetDir) } - pkgSources, err := resolvePkgSources(options.Archives, options.Selection) + pkgSources, err := resolvePkgSources(options.Archives, options.Stores, options.Selection) if err != nil { return err } @@ -250,6 +250,12 @@ func Run(options *RunOptions) error { if reader == nil { continue } + // src := pkgSources[slice.Package] + // Store packages are distributed as plain tarballs, whose extraction + // is not yet implemented. Fail until the format support is in place. + // if src.kind == sourceStore { + // return fmt.Errorf("cannot extract package %q from store: store packages are not yet supported", src.pkg.Name) + // } err := tarball.Extract(reader, &tarball.ExtractOptions{ Package: slice.Package, Extract: extract[slice.Package], @@ -363,7 +369,8 @@ func Run(options *RunOptions) error { } func generateManifests(targetDir string, selection *setup.Selection, - report *manifestutil.Report, pkgInfos []*archive.PackageInfo) error { + report *manifestutil.Report, pkgInfos []*archive.PackageInfo, +) error { manifestSlices := manifestutil.FindPaths(selection.Slices) if len(manifestSlices) == 0 { // Nothing to do. @@ -468,9 +475,9 @@ func createFile(targetDir, relPath string, pathInfo setup.PathInfo) (*fsutil.Ent targetMode := pathInfo.Mode if targetMode == 0 { if pathInfo.Kind == setup.DirPath { - targetMode = 0755 + targetMode = 0o755 } else { - targetMode = 0644 + targetMode = 0o644 } } @@ -504,10 +511,9 @@ func createFile(targetDir, relPath string, pathInfo setup.PathInfo) (*fsutil.Ent // resolvePkgSources determines the source for each package in the selection. // For archive packages it selects the highest priority archive containing the // package unless a particular archive is pinned within the slice definition -// file. For store packages it records a fetch function that returns an error -// until store support is implemented. It returns a map of pkgSource indexed by -// package names. -func resolvePkgSources(archives map[string]archive.Archive, selection *setup.Selection) (map[string]pkgSource, error) { +// file. For store packages it records the opened store handle. It returns a map +// of pkgSourceInfo indexed by package names. +func resolvePkgSources(archives map[string]archive.Archive, stores map[string]store.Store, selection *setup.Selection) (map[string]pkgSource, error) { sortedArchives := make([]*setup.Archive, 0, len(selection.Release.Archives)) for _, archive := range selection.Release.Archives { if archive.Priority < 0 { @@ -531,7 +537,7 @@ func resolvePkgSources(archives map[string]archive.Archive, selection *setup.Sel pkgSources[pkg.Name] = pkgSource{ // TODO: set the arch when implementing fetching from the store. fetch: func() (io.ReadSeekCloser, *archive.PackageInfo, error) { - return nil, nil, fmt.Errorf("cannot fetch package %q from store %q: not implemented", pkg.Name, pkg.Store) + return stores[pkg.Store].Fetch(pkg.RealName, pkg.DefaultTrack, "") }, } continue diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 954602de4..72d287129 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -18,6 +18,7 @@ import ( "github.com/canonical/chisel/internal/manifestutil" "github.com/canonical/chisel/internal/setup" "github.com/canonical/chisel/internal/slicer" + "github.com/canonical/chisel/internal/store" "github.com/canonical/chisel/internal/testutil" "github.com/canonical/chisel/public/manifest" ) @@ -1982,6 +1983,14 @@ var slicerTests = []slicerTest{{ summary: "Store package fetching not yet implemented", slices: []setup.SliceKey{{"test-package", "myslice"}, {"bin-store-pkg", "myslice"}}, arch: "amd64", + pkgs: []*testutil.TestPackage{{ + Name: "test-package", + Data: testutil.PackageData["test-package"], + }, { + Name: "store-pkg", + Store: "bin", + Data: testutil.PackageData["test-package"], + }}, release: map[string]string{ "chisel.yaml": testutil.DefaultChiselYamlWithStores, "slices/mydir/test-package.yaml": ` @@ -2109,6 +2118,9 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { for name, setupArchive := range release.Archives { pkgs := make(map[string]*testutil.TestPackage) for _, pkg := range test.pkgs { + if pkg.Store != "" { + continue + } if len(pkg.Archives) == 0 || slices.Contains(pkg.Archives, name) { pkgs[pkg.Name] = pkg } @@ -2127,9 +2139,21 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { archives[name] = archive } + stores := map[string]store.Store{} + for name := range release.Stores { + pkgs := make(map[string]*testutil.TestPackage) + for _, pkg := range test.pkgs { + if pkg.Store == name { + pkgs[pkg.Name] = pkg + } + } + stores[name] = &testutil.TestStore{Packages: pkgs} + } + options := slicer.RunOptions{ Selection: selection, Archives: archives, + Stores: stores, TargetDir: c.MkDir(), } if test.hackopt != nil { diff --git a/internal/store/store.go b/internal/store/store.go index b842fb086..a8fd8cf81 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -11,23 +11,16 @@ import ( "strings" "time" + "github.com/canonical/chisel/internal/archive" "github.com/canonical/chisel/internal/cache" "github.com/canonical/chisel/internal/deb" ) // Store provides access to packages from the Snapcraft store API. type Store interface { - Fetch(name, track, risk string) (io.ReadSeekCloser, *StorePackageInfo, error) + Fetch(name, track, risk string) (io.ReadSeekCloser, *archive.PackageInfo, error) Exists(name, track, risk string) bool - Info(name, track, risk string) (*StorePackageInfo, error) -} - -// StorePackageInfo holds metadata about a package. -type StorePackageInfo struct { - Name string - Version string - Revision int - SHA384 string + Info(name, track, risk string) (*archive.PackageInfo, error) } type Options struct { @@ -40,6 +33,10 @@ type storeKind string const storeKindBin storeKind = "bin" +// defaultRisk is the channel risk used when a specific risk has not been +// requested. +const defaultRisk = "stable" + type binStore struct { options Options cache *cache.Cache @@ -168,8 +165,12 @@ func (s *binStore) fetchBinInfo(name string) (*binInfoResponse, error) { } // selectRevision finds the channel-map entry matching the requested track, -// risk, and architecture, returning its revision. +// risk, and architecture, returning its revision. An empty risk falls back to +// defaultRisk. func selectRevision(info *binInfoResponse, arch, track, risk string) (*binRevision, error) { + if risk == "" { + risk = defaultRisk + } for i := range info.ChannelMap { entry := &info.ChannelMap[i] if entry.Channel.Track != track || entry.Channel.Risk != risk { @@ -213,7 +214,7 @@ func validateDownloadURL(downloadURL string) error { return fmt.Errorf("bin download URL has untrusted host %q", u.Host) } -func (s *binStore) Info(name, track, risk string) (*StorePackageInfo, error) { +func (s *binStore) Info(name, track, risk string) (*archive.PackageInfo, error) { resp, err := s.fetchBinInfo(name) if err != nil { return nil, err @@ -222,7 +223,7 @@ func (s *binStore) Info(name, track, risk string) (*StorePackageInfo, error) { if err != nil { return nil, err } - return &StorePackageInfo{ + return &archive.PackageInfo{ Name: name, Version: rev.Version, Revision: rev.Revision, @@ -235,7 +236,7 @@ func (s *binStore) Exists(name, track, risk string) bool { return err == nil } -func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *StorePackageInfo, error) { +func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive.PackageInfo, error) { logf("Fetching bin %s %s/%s ...", name, track, risk) resp, err := s.fetchBinInfo(name) @@ -247,7 +248,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *StorePac return nil, nil, err } - info := &StorePackageInfo{ + info := &archive.PackageInfo{ Name: name, Version: rev.Version, Revision: rev.Revision, diff --git a/internal/testutil/archive.go b/internal/testutil/archive.go index d06fd1b0c..ccac36890 100644 --- a/internal/testutil/archive.go +++ b/internal/testutil/archive.go @@ -13,15 +13,6 @@ type TestArchive struct { Packages map[string]*TestPackage } -type TestPackage struct { - Name string - Version string - Hash string - Arch string - Data []byte - Archives []string -} - func (a *TestArchive) Options() *archive.Options { return &a.Opts } diff --git a/internal/testutil/package.go b/internal/testutil/package.go new file mode 100644 index 000000000..561eb105c --- /dev/null +++ b/internal/testutil/package.go @@ -0,0 +1,17 @@ +package testutil + +// TestPackage is a source-agnostic package fixture that can be served by either +// a TestArchive or a TestStore. +type TestPackage struct { + Name string + Version string + Hash string + Arch string + Data []byte + // Archives lists the archives the package belongs to. It is only relevant + // for archive-backed packages. + Archives []string + // Store names the store the package is served from. When set, the package + // is store-backed rather than archive-backed. + Store string +} diff --git a/internal/testutil/store.go b/internal/testutil/store.go new file mode 100644 index 000000000..b8e96a850 --- /dev/null +++ b/internal/testutil/store.go @@ -0,0 +1,43 @@ +package testutil + +import ( + "bytes" + "fmt" + "io" + + "github.com/canonical/chisel/internal/archive" +) + +type TestStore struct { + Packages map[string]*TestPackage +} + +func (s *TestStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive.PackageInfo, error) { + pkg, ok := s.Packages[name] + if !ok { + return nil, nil, fmt.Errorf("cannot find package %q in store", name) + } + info := &archive.PackageInfo{ + Name: pkg.Name, + Version: pkg.Version, + SHA384: pkg.Hash, + } + return ReadSeekNopCloser(bytes.NewReader(pkg.Data)), info, nil +} + +func (s *TestStore) Exists(name, track, risk string) bool { + _, ok := s.Packages[name] + return ok +} + +func (s *TestStore) Info(name, track, risk string) (*archive.PackageInfo, error) { + pkg, ok := s.Packages[name] + if !ok { + return nil, fmt.Errorf("cannot find package %q in store", name) + } + return &archive.PackageInfo{ + Name: pkg.Name, + Version: pkg.Version, + SHA384: pkg.Hash, + }, nil +} From 987d776263de9c8cc85b18ca70fcf314a76fa9e1 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Mon, 22 Jun 2026 18:22:03 +0200 Subject: [PATCH 08/20] fix: review --- internal/slicer/slicer.go | 6 +++++- internal/store/store.go | 33 ++++++++++++++++++++++----------- internal/store/store_test.go | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 12 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 365d3d66e..5cc7ecb40 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -534,10 +534,14 @@ func resolvePkgSources(archives map[string]archive.Archive, stores map[string]st } pkg := selection.Release.Packages[s.Package] if pkg.Store != "" { + storeHandle := stores[pkg.Store] + if storeHandle == nil { + return nil, fmt.Errorf("internal error: no store handle for store %q", pkg.Store) + } pkgSources[pkg.Name] = pkgSource{ // TODO: set the arch when implementing fetching from the store. fetch: func() (io.ReadSeekCloser, *archive.PackageInfo, error) { - return stores[pkg.Store].Fetch(pkg.RealName, pkg.DefaultTrack, "") + return storeHandle.Fetch(pkg.RealName, pkg.DefaultTrack, "") }, } continue diff --git a/internal/store/store.go b/internal/store/store.go index a8fd8cf81..128403914 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -164,13 +164,17 @@ func (s *binStore) fetchBinInfo(name string) (*binInfoResponse, error) { return &info, nil } -// selectRevision finds the channel-map entry matching the requested track, -// risk, and architecture, returning its revision. An empty risk falls back to -// defaultRisk. -func selectRevision(info *binInfoResponse, arch, track, risk string) (*binRevision, error) { +func resolveRisk(risk string) string { if risk == "" { - risk = defaultRisk + return defaultRisk } + return risk +} + +// selectRevision finds the channel-map entry matching the requested track, +// risk, and architecture, returning its revision. It fails if the matched +// revision has no download digest. +func selectRevision(info *binInfoResponse, arch, track, risk string) (*binRevision, error) { for i := range info.ChannelMap { entry := &info.ChannelMap[i] if entry.Channel.Track != track || entry.Channel.Risk != risk { @@ -179,6 +183,9 @@ func selectRevision(info *binInfoResponse, arch, track, risk string) (*binRevisi if entry.Channel.Platform.Architecture != arch { continue } + if entry.Revision.Download.SHA3384 == "" { + return nil, fmt.Errorf("bin %q has no download digest", info.Name) + } return &entry.Revision, nil } return nil, fmt.Errorf("bin %q has no %s/%s release for architecture %q", info.Name, track, risk, arch) @@ -206,15 +213,17 @@ func validateDownloadURL(downloadURL string) error { if u.Scheme != "https" { return fmt.Errorf("bin download URL must use HTTPS: %q", downloadURL) } - for _, host := range allowedDownloadHosts { - if u.Host == host || strings.HasSuffix(u.Host, "."+host) { + host := strings.ToLower(u.Hostname()) + for _, allowed := range allowedDownloadHosts { + if host == allowed || strings.HasSuffix(host, "."+allowed) { return nil } } - return fmt.Errorf("bin download URL has untrusted host %q", u.Host) + return fmt.Errorf("bin download URL has untrusted host %q", host) } func (s *binStore) Info(name, track, risk string) (*archive.PackageInfo, error) { + risk = resolveRisk(risk) resp, err := s.fetchBinInfo(name) if err != nil { return nil, err @@ -237,6 +246,7 @@ func (s *binStore) Exists(name, track, risk string) bool { } func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive.PackageInfo, error) { + risk = resolveRisk(risk) logf("Fetching bin %s %s/%s ...", name, track, risk) resp, err := s.fetchBinInfo(name) @@ -248,6 +258,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. return nil, nil, err } + digest := rev.Download.SHA3384 info := &archive.PackageInfo{ Name: name, Version: rev.Version, @@ -256,7 +267,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. } // Check cache first. - reader, err := s.cache.Open(cache.SHA384, rev.Download.SHA3384) + reader, err := s.cache.Open(cache.SHA384, digest) if err == nil { logf("Using cached bin %s", name) return reader, info, nil @@ -284,7 +295,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. return nil, nil, fmt.Errorf("cannot download bin %q: %v", name, httpResp.Status) } - writer := s.cache.Create(cache.SHA384, rev.Download.SHA3384) + writer := s.cache.Create(cache.SHA384, digest) defer writer.Close() _, err = io.Copy(writer, httpResp.Body) @@ -295,7 +306,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. return nil, nil, fmt.Errorf("cannot fetch bin %q: %v", name, err) } - reader, err = s.cache.Open(cache.SHA384, rev.Download.SHA3384) + reader, err = s.cache.Open(cache.SHA384, digest) if err != nil { return nil, nil, err } diff --git a/internal/store/store_test.go b/internal/store/store_test.go index e1525c0bd..e6ba5590a 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -112,6 +112,8 @@ func (s *storeSuite) TestValidateDownloadURL(c *C) { {"https://api.snapcraft.io/v2/bins/foo", ""}, {"https://api.staging.snapcraft.io/v2/bins/foo", ""}, {"https://sub.storage.snapcraftcontent.com/bins/foo.tar.xz", ""}, + {"https://storage.snapcraftcontent.com:443/bins/foo.tar.xz", ""}, + {"https://Storage.SnapcraftContent.Com/bins/foo.tar.xz", ""}, { "http://storage.snapcraftcontent.com/bins/foo.tar.xz", `bin download URL must use HTTPS: "http://storage.snapcraftcontent.com/bins/foo.tar.xz"`, @@ -120,6 +122,10 @@ func (s *storeSuite) TestValidateDownloadURL(c *C) { "https://evil.example.com/bins/foo.tar.xz", `bin download URL has untrusted host "evil.example.com"`, }, + { + "https://Evil.Example.Com/bins/foo.tar.xz", + `bin download URL has untrusted host "evil.example.com"`, + }, { "https://storage.snapcraftcontent.com.evil.com/bins/foo.tar.xz", `bin download URL has untrusted host "storage.snapcraftcontent.com.evil.com"`, @@ -211,6 +217,11 @@ var infoTests = []infoTest{{ ] }`, info: &store.StorePackageInfo{Name: "curl", Version: "8.5.0", Revision: 42, SHA384: "amd64hash"}, +}, { + summary: "Missing download digest", + status: 200, + body: string(makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "")), + error: `bin "curl" has no download digest`, }} func (s *storeSuite) TestInfo(c *C) { @@ -407,6 +418,28 @@ func (s *storeSuite) TestFetchInvalidDownloadURL(c *C) { c.Assert(err, ErrorMatches, `bin download URL must use HTTPS: "http://evil.example.com/bins/curl.tar.xz"`) } +func (s *storeSuite) TestFetchMissingDigest(c *C) { + // The store omits the sha3-384 digest from the response. + infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "") + + s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(infoBody)), + }, nil + } + + src, err := store.Open(&store.Options{ + Arch: "amd64", + CacheDir: s.cacheDir, + Kind: "bin", + }) + c.Assert(err, IsNil) + + _, _, err = src.Fetch("curl", "latest", "stable") + c.Assert(err, ErrorMatches, `bin "curl" has no download digest`) +} + func (s *storeSuite) TestStagingEnvVar(c *C) { s.envRestore() s.envRestore = fakeEnv("1") From 636052ba37316ccdd566ca5206efae229e859a55 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 23 Jun 2026 09:14:26 +0200 Subject: [PATCH 09/20] fix: improve naming and add more tests --- internal/store/store.go | 14 +++++++------- internal/store/store_test.go | 16 +++++++++++++++- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/internal/store/store.go b/internal/store/store.go index 128403914..5544d3da4 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -118,9 +118,9 @@ type binRevision struct { } type binDownload struct { - URL string `json:"url"` - SHA3384 string `json:"sha3-384"` - Size int64 `json:"size"` + URL string `json:"url"` + SHA384 string `json:"sha3-384"` + Size int64 `json:"size"` } func (s *binStore) fetchBinInfo(name string) (*binInfoResponse, error) { @@ -183,7 +183,7 @@ func selectRevision(info *binInfoResponse, arch, track, risk string) (*binRevisi if entry.Channel.Platform.Architecture != arch { continue } - if entry.Revision.Download.SHA3384 == "" { + if entry.Revision.Download.SHA384 == "" { return nil, fmt.Errorf("bin %q has no download digest", info.Name) } return &entry.Revision, nil @@ -236,7 +236,7 @@ func (s *binStore) Info(name, track, risk string) (*archive.PackageInfo, error) Name: name, Version: rev.Version, Revision: rev.Revision, - SHA384: rev.Download.SHA3384, + SHA384: rev.Download.SHA384, }, nil } @@ -258,12 +258,12 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. return nil, nil, err } - digest := rev.Download.SHA3384 + digest := rev.Download.SHA384 info := &archive.PackageInfo{ Name: name, Version: rev.Version, Revision: rev.Revision, - SHA384: rev.Download.SHA3384, + SHA384: rev.Download.SHA384, } // Check cache first. diff --git a/internal/store/store_test.go b/internal/store/store_test.go index e6ba5590a..2131f4b78 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -167,6 +167,7 @@ func (s *storeSuite) TestOpenArchValidation(c *C) { type infoTest struct { summary string + risk string status int statusText string body string @@ -176,32 +177,44 @@ type infoTest struct { var infoTests = []infoTest{{ summary: "Successful info", + risk: "stable", + status: 200, + body: string(makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123")), + info: &store.StorePackageInfo{Name: "curl", Version: "8.5.0", Revision: 42, SHA384: "abc123"}, +}, { + summary: "Defaults to stable risk when unspecified", + risk: "", status: 200, body: string(makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123")), info: &store.StorePackageInfo{Name: "curl", Version: "8.5.0", Revision: 42, SHA384: "abc123"}, }, { summary: "Package not found", + risk: "stable", status: 404, body: "not found", error: `bin "curl" not found`, }, { summary: "No release for the requested architecture", + risk: "stable", status: 200, body: string(makeBinInfoBody("curl", "latest", "stable", "arm64", "8.5.0", 42, "abc123")), error: `bin "curl" has no latest/stable release for architecture "amd64"`, }, { summary: "Server error", + risk: "stable", status: 500, statusText: "500 Internal Server Error", body: "boom", error: "cannot fetch from bin store: 500 Internal Server Error", }, { summary: "Malformed response body", + risk: "stable", status: 200, body: "not json", error: "cannot decode bin store response: .*", }, { summary: "Selects the entry matching the requested architecture", + risk: "stable", status: 200, body: `{ "name": "curl", @@ -219,6 +232,7 @@ var infoTests = []infoTest{{ info: &store.StorePackageInfo{Name: "curl", Version: "8.5.0", Revision: 42, SHA384: "amd64hash"}, }, { summary: "Missing download digest", + risk: "stable", status: 200, body: string(makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "")), error: `bin "curl" has no download digest`, @@ -243,7 +257,7 @@ func (s *storeSuite) TestInfo(c *C) { }) c.Assert(err, IsNil) - info, err := src.Info("curl", "latest", "stable") + info, err := src.Info("curl", "latest", test.risk) if test.error != "" { c.Assert(err, ErrorMatches, test.error) continue From 3e7411a6e3fefa8b580fe3ed8eeafc952fa2f788 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 23 Jun 2026 10:54:01 +0200 Subject: [PATCH 10/20] fix: properly build track value with store version --- cmd/chisel/cmd_cut.go | 1 + internal/slicer/slicer.go | 6 +++++- internal/slicer/slicer_test.go | 9 +++++++-- internal/store/store.go | 6 ++++++ internal/testutil/store.go | 6 ++++++ 5 files changed, 25 insertions(+), 3 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 1ba1f2033..8cdd11ce6 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -128,6 +128,7 @@ func (cmd *cmdCut) Execute(args []string) error { Arch: cmd.Arch, CacheDir: cache.DefaultDir("chisel"), Kind: storeInfo.Kind, + Version: storeInfo.Version, }) if err != nil { return err diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 5cc7ecb40..6e9852731 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -538,10 +538,14 @@ func resolvePkgSources(archives map[string]archive.Archive, stores map[string]st if storeHandle == nil { return nil, fmt.Errorf("internal error: no store handle for store %q", pkg.Store) } + // The package metadata returned by the store is not yet recorded + // in the manifest; this is handled in a subsequent change. + // Risk is left unspecified for now; the store applies its default. + track := pkg.DefaultTrack + "-" + storeHandle.Options().Version pkgSources[pkg.Name] = pkgSource{ // TODO: set the arch when implementing fetching from the store. fetch: func() (io.ReadSeekCloser, *archive.PackageInfo, error) { - return storeHandle.Fetch(pkg.RealName, pkg.DefaultTrack, "") + return storeHandle.Fetch(pkg.RealName, track, "") }, } continue diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index 72d287129..c6eab7883 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -2140,14 +2140,19 @@ func runSlicerTests(s *S, c *C, tests []slicerTest) { } stores := map[string]store.Store{} - for name := range release.Stores { + for name, relStore := range release.Stores { pkgs := make(map[string]*testutil.TestPackage) for _, pkg := range test.pkgs { if pkg.Store == name { pkgs[pkg.Name] = pkg } } - stores[name] = &testutil.TestStore{Packages: pkgs} + stores[name] = &testutil.TestStore{ + Packages: pkgs, + Opts: store.Options{ + Version: relStore.Version, + }, + } } options := slicer.RunOptions{ diff --git a/internal/store/store.go b/internal/store/store.go index 5544d3da4..91b746fab 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -18,6 +18,7 @@ import ( // Store provides access to packages from the Snapcraft store API. type Store interface { + Options() *Options Fetch(name, track, risk string) (io.ReadSeekCloser, *archive.PackageInfo, error) Exists(name, track, risk string) bool Info(name, track, risk string) (*archive.PackageInfo, error) @@ -27,6 +28,7 @@ type Options struct { Arch string CacheDir string Kind string + Version string } type storeKind string @@ -222,6 +224,10 @@ func validateDownloadURL(downloadURL string) error { return fmt.Errorf("bin download URL has untrusted host %q", host) } +func (s *binStore) Options() *Options { + return &s.options +} + func (s *binStore) Info(name, track, risk string) (*archive.PackageInfo, error) { risk = resolveRisk(risk) resp, err := s.fetchBinInfo(name) diff --git a/internal/testutil/store.go b/internal/testutil/store.go index b8e96a850..b985a6aa6 100644 --- a/internal/testutil/store.go +++ b/internal/testutil/store.go @@ -6,12 +6,18 @@ import ( "io" "github.com/canonical/chisel/internal/archive" + "github.com/canonical/chisel/internal/store" ) type TestStore struct { + Opts store.Options Packages map[string]*TestPackage } +func (s *TestStore) Options() *store.Options { + return &s.Opts +} + func (s *TestStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive.PackageInfo, error) { pkg, ok := s.Packages[name] if !ok { From 7d54d0430c711fe028a44a248d7efaf322a293d9 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 23 Jun 2026 14:02:32 +0200 Subject: [PATCH 11/20] feat: refine download host validation --- internal/store/store.go | 56 +++++++++++++++++------------------- internal/store/store_test.go | 52 +++++++++++++++++++++------------ 2 files changed, 59 insertions(+), 49 deletions(-) diff --git a/internal/store/store.go b/internal/store/store.go index 91b746fab..6115add1c 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -16,7 +16,7 @@ import ( "github.com/canonical/chisel/internal/deb" ) -// Store provides access to packages from the Snapcraft store API. +// Store provides access to packages from the Store API. type Store interface { Options() *Options Fetch(name, track, risk string) (io.ReadSeekCloser, *archive.PackageInfo, error) @@ -35,20 +35,21 @@ type storeKind string const storeKindBin storeKind = "bin" -// defaultRisk is the channel risk used when a specific risk has not been -// requested. const defaultRisk = "stable" type binStore struct { - options Options - cache *cache.Cache - apiURL string + options Options + cache *cache.Cache + apiURL string + downloadHost string } const ( - binAPIBase = "https://api.snapcraft.io/v2/bins" - binAPIStaging = "https://api.staging.snapcraft.io/v2/bins" - binStagingEnvVar = "CHISEL_BIN_STAGING" + binAPIBase = "https://api.snapcraft.io/v2/bins" + binAPIBaseStaging = "https://api.staging.snapcraft.io/v2/bins" + binDownloadHost = "api.snapcraft.io" + binDownloadHostStaging = "api.staging.snapcraft.io" + binStagingEnvVar = "CHISEL_BIN_STAGING" ) var httpClient = &http.Client{ @@ -77,13 +78,16 @@ func Open(options *Options) (Store, error) { switch storeKind(options.Kind) { case storeKindBin: apiURL := binAPIBase + downloadHost := binDownloadHost if os.Getenv(binStagingEnvVar) != "" { - apiURL = binAPIStaging + apiURL = binAPIBaseStaging + downloadHost = binDownloadHostStaging } return &binStore{ - options: *options, - cache: &cache.Cache{Dir: options.CacheDir}, - apiURL: apiURL, + options: *options, + cache: &cache.Cache{Dir: options.CacheDir}, + apiURL: apiURL, + downloadHost: downloadHost, }, nil default: return nil, fmt.Errorf("unsupported store kind %q", options.Kind) @@ -198,16 +202,9 @@ func selectRevision(info *binInfoResponse, arch, track, risk string) (*binRevisi // store API URL path when interpolated into it. var nameExp = regexp.MustCompile(`^[a-z0-9][a-z0-9+.-]*$`) -// allowedDownloadHosts lists the hosts from which bin downloads are permitted. -var allowedDownloadHosts = []string{ - "api.snapcraft.io", - "api.staging.snapcraft.io", - "storage.snapcraftcontent.com", -} - -// validateDownloadURL checks that the download URL is HTTPS and from an +// validateDownloadURL checks that the download URL is HTTPS and from the // allowed host. -func validateDownloadURL(downloadURL string) error { +func validateDownloadURL(downloadURL, allowedHost string) error { u, err := url.Parse(downloadURL) if err != nil { return fmt.Errorf("cannot parse bin download URL: %v", err) @@ -216,10 +213,8 @@ func validateDownloadURL(downloadURL string) error { return fmt.Errorf("bin download URL must use HTTPS: %q", downloadURL) } host := strings.ToLower(u.Hostname()) - for _, allowed := range allowedDownloadHosts { - if host == allowed || strings.HasSuffix(host, "."+allowed) { - return nil - } + if host == allowedHost || strings.HasSuffix(host, "."+allowedHost) { + return nil } return fmt.Errorf("bin download URL has untrusted host %q", host) } @@ -272,8 +267,9 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. SHA384: rev.Download.SHA384, } + const digestKind = cache.SHA256 // Check cache first. - reader, err := s.cache.Open(cache.SHA384, digest) + reader, err := s.cache.Open(digestKind, digest) if err == nil { logf("Using cached bin %s", name) return reader, info, nil @@ -282,7 +278,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. } // Download the bin. - err = validateDownloadURL(rev.Download.URL) + err = validateDownloadURL(rev.Download.URL, s.downloadHost) if err != nil { return nil, nil, err } @@ -301,7 +297,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. return nil, nil, fmt.Errorf("cannot download bin %q: %v", name, httpResp.Status) } - writer := s.cache.Create(cache.SHA384, digest) + writer := s.cache.Create(digestKind, digest) defer writer.Close() _, err = io.Copy(writer, httpResp.Body) @@ -312,7 +308,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. return nil, nil, fmt.Errorf("cannot fetch bin %q: %v", name, err) } - reader, err = s.cache.Open(cache.SHA384, digest) + reader, err = s.cache.Open(digestKind, writer.Digest()) if err != nil { return nil, nil, err } diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 2131f4b78..6d29fb167 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -74,7 +74,7 @@ func sha384Hash(data []byte) string { } func makeBinInfoBody(name, track, risk, arch, version string, revision int, sha384 string) []byte { - downloadURL := "https://storage.snapcraftcontent.com/bins/" + name + ".tar.xz" + downloadURL := "https://api.snapcraft.io/api/v1/bins/download/" + name + ".bin" return makeBinInfoBodyWithURL(name, track, risk, arch, version, revision, sha384, downloadURL) } @@ -105,35 +105,49 @@ func makeBinInfoBodyWithURL(name, track, risk, arch, version string, revision in func (s *storeSuite) TestValidateDownloadURL(c *C) { tests := []struct { - url string - error string + url string + allowedHost string + error string }{ - {"https://storage.snapcraftcontent.com/bins/foo.tar.xz", ""}, - {"https://api.snapcraft.io/v2/bins/foo", ""}, - {"https://api.staging.snapcraft.io/v2/bins/foo", ""}, - {"https://sub.storage.snapcraftcontent.com/bins/foo.tar.xz", ""}, - {"https://storage.snapcraftcontent.com:443/bins/foo.tar.xz", ""}, - {"https://Storage.SnapcraftContent.Com/bins/foo.tar.xz", ""}, + {"https://api.snapcraft.io/api/v1/bins/download/foo.bin", "api.snapcraft.io", ""}, + // Staging host is rejected when the production host is allowed. + { + "https://api.staging.snapcraft.io/api/v1/bins/download/foo.bin", + "api.snapcraft.io", + `bin download URL has untrusted host "api.staging.snapcraft.io"`, + }, + // Staging host is allowed when it is the allowed host. + {"https://api.staging.snapcraft.io/api/v1/bins/download/foo.bin", "api.staging.snapcraft.io", ""}, + // Production API host is rejected when the staging host is allowed. + { + "https://api.snapcraft.io/api/v1/bins/download/foo.bin", + "api.staging.snapcraft.io", + `bin download URL has untrusted host "api.snapcraft.io"`, + }, { - "http://storage.snapcraftcontent.com/bins/foo.tar.xz", - `bin download URL must use HTTPS: "http://storage.snapcraftcontent.com/bins/foo.tar.xz"`, + "http://api.snapcraft.io/api/v1/bins/download/foo.bin", + "api.snapcraft.io", + `bin download URL must use HTTPS: "http://api.snapcraft.io/api/v1/bins/download/foo.bin"`, }, { - "https://evil.example.com/bins/foo.tar.xz", + "https://evil.example.com/api/v1/bins/download/foo.bin", + "api.snapcraft.io", `bin download URL has untrusted host "evil.example.com"`, }, { - "https://Evil.Example.Com/bins/foo.tar.xz", + "https://Evil.Example.Com/api/v1/bins/download/foo.bin", + "api.snapcraft.io", `bin download URL has untrusted host "evil.example.com"`, }, { - "https://storage.snapcraftcontent.com.evil.com/bins/foo.tar.xz", - `bin download URL has untrusted host "storage.snapcraftcontent.com.evil.com"`, + "https://api.snapcraft.io.evil.com/api/v1/bins/download/foo.bin", + "api.snapcraft.io", + `bin download URL has untrusted host "api.snapcraft.io.evil.com"`, }, - {"://invalid-url", `cannot parse bin download URL: .*`}, + {"://invalid-url", "api.snapcraft.io", `cannot parse bin download URL: .*`}, } for _, test := range tests { - err := store.ValidateDownloadURL(test.url) + err := store.ValidateDownloadURL(test.url, test.allowedHost) if test.error == "" { c.Assert(err, IsNil) } else { @@ -221,11 +235,11 @@ var infoTests = []infoTest{{ "channel-map": [ { "channel": {"name": "latest/stable", "risk": "stable", "track": "latest", "platform": {"architecture": "arm64"}}, - "revision": {"version": "8.0.0", "revision": 1, "download": {"url": "https://storage.snapcraftcontent.com/bins/curl.tar.xz", "sha3-384": "arm64hash", "size": 1024}} + "revision": {"version": "8.0.0", "revision": 1, "download": {"url": "https://api.snapcraft.io/api/v1/bins/download/curl.bin", "sha3-384": "arm64hash", "size": 1024}} }, { "channel": {"name": "latest/stable", "risk": "stable", "track": "latest", "platform": {"architecture": "amd64"}}, - "revision": {"version": "8.5.0", "revision": 42, "download": {"url": "https://storage.snapcraftcontent.com/bins/curl.tar.xz", "sha3-384": "amd64hash", "size": 1024}} + "revision": {"version": "8.5.0", "revision": 42, "download": {"url": "https://api.snapcraft.io/api/v1/bins/download/curl.bin", "sha3-384": "amd64hash", "size": 1024}} } ] }`, From ee15b404ec34d206daebce20baed83aecbace7a9 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 23 Jun 2026 15:18:29 +0200 Subject: [PATCH 12/20] feat: use revisions/resolve endpoint --- internal/store/store.go | 139 +++++++++++++++++--------------- internal/store/store_test.go | 150 ++++++++++++++++++++--------------- 2 files changed, 160 insertions(+), 129 deletions(-) diff --git a/internal/store/store.go b/internal/store/store.go index 6115add1c..5630ea1d2 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -1,6 +1,7 @@ package store import ( + "bytes" "encoding/json" "fmt" "io" @@ -45,8 +46,8 @@ type binStore struct { } const ( - binAPIBase = "https://api.snapcraft.io/v2/bins" - binAPIBaseStaging = "https://api.staging.snapcraft.io/v2/bins" + binAPIBase = "https://api.snapcraft.io/v2" + binAPIBaseStaging = "https://api.staging.snapcraft.io/v2" binDownloadHost = "api.snapcraft.io" binDownloadHostStaging = "api.staging.snapcraft.io" binStagingEnvVar = "CHISEL_BIN_STAGING" @@ -94,23 +95,37 @@ func Open(options *Options) (Store, error) { } } -// binInfoResponse represents the JSON response from the bin store info endpoint. -type binInfoResponse struct { - Name string `json:"name"` - PackageID string `json:"package-id"` - ChannelMap []binChannelMap `json:"channel-map"` +// resolveRequest is the body sent to the revisions/resolve endpoint. +type resolveRequest struct { + Packages []resolvePackage `json:"packages"` } -type binChannelMap struct { - Channel binChannel `json:"channel"` - Revision binRevision `json:"revision"` +type resolvePackage struct { + InstanceKey string `json:"instance-key"` + Namespace string `json:"namespace"` + Name string `json:"name"` + Channel string `json:"channel"` + Platform binPlatform `json:"platform"` +} + +type resolveResponse struct { + PackageResults []resolveResult `json:"package-results"` +} + +type resolveResult struct { + InstanceKey string `json:"instance-key"` + Status string `json:"status"` + Error *resolveError `json:"error"` + Result *resolveEntry `json:"result"` +} + +type resolveError struct { + Code string `json:"code"` + Message string `json:"message"` } -type binChannel struct { - Name string `json:"name"` - Risk string `json:"risk"` - Track string `json:"track"` - Platform binPlatform `json:"platform"` +type resolveEntry struct { + Revision binRevision `json:"revision"` } type binPlatform struct { @@ -129,7 +144,10 @@ type binDownload struct { Size int64 `json:"size"` } -func (s *binStore) fetchBinInfo(name string) (*binInfoResponse, error) { +// resolveRevision resolves a single package revision via the store API. It +// returns the matching revision or an error if the package is not found or has +// no release for the requested channel and architecture. +func (s *binStore) resolveRevision(name, track, risk string) (*binRevision, error) { if !nameExp.MatchString(name) { return nil, fmt.Errorf("invalid package name %q", name) } @@ -137,15 +155,27 @@ func (s *binStore) fetchBinInfo(name string) (*binInfoResponse, error) { if err != nil { return nil, fmt.Errorf("internal error: cannot parse bin store URL: %v", err) } - u = u.JoinPath("info", name) - u.RawQuery = url.Values{ - "fields": {"download,version,revision,channel-map"}, - }.Encode() + u = u.JoinPath("revisions", "resolve") + + reqBody, err := json.Marshal(resolveRequest{ + Packages: []resolvePackage{{ + InstanceKey: name, + Namespace: string(storeKindBin), + Name: name, + Channel: track + "/" + risk, + Platform: binPlatform{Architecture: s.options.Arch}, + }}, + }) + if err != nil { + return nil, fmt.Errorf("internal error: cannot encode resolve request: %v", err) + } - req, err := http.NewRequest("GET", u.String(), nil) + req, err := http.NewRequest("POST", u.String(), bytes.NewReader(reqBody)) if err != nil { return nil, fmt.Errorf("cannot create HTTP request: %v", err) } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/json") resp, err := httpDo(req) if err != nil { @@ -153,48 +183,31 @@ func (s *binStore) fetchBinInfo(name string) (*binInfoResponse, error) { } defer resp.Body.Close() - switch resp.StatusCode { - case 200: - // ok - case 404: - return nil, fmt.Errorf("bin %q not found", name) - default: + if resp.StatusCode != 200 { return nil, fmt.Errorf("cannot fetch from bin store: %v", resp.Status) } - var info binInfoResponse - err = json.NewDecoder(resp.Body).Decode(&info) + var res resolveResponse + err = json.NewDecoder(resp.Body).Decode(&res) if err != nil { return nil, fmt.Errorf("cannot decode bin store response: %v", err) } - return &info, nil -} -func resolveRisk(risk string) string { - if risk == "" { - return defaultRisk + if len(res.PackageResults) == 0 { + return nil, fmt.Errorf("bin %q not found", name) } - return risk -} - -// selectRevision finds the channel-map entry matching the requested track, -// risk, and architecture, returning its revision. It fails if the matched -// revision has no download digest. -func selectRevision(info *binInfoResponse, arch, track, risk string) (*binRevision, error) { - for i := range info.ChannelMap { - entry := &info.ChannelMap[i] - if entry.Channel.Track != track || entry.Channel.Risk != risk { - continue - } - if entry.Channel.Platform.Architecture != arch { - continue - } - if entry.Revision.Download.SHA384 == "" { - return nil, fmt.Errorf("bin %q has no download digest", info.Name) + result := &res.PackageResults[0] + if result.Status != "ok" || result.Result == nil { + if result.Error != nil { + return nil, fmt.Errorf("bin %q not found: %s", name, result.Error.Message) } - return &entry.Revision, nil + return nil, fmt.Errorf("bin %q not found", name) + } + rev := &result.Result.Revision + if rev.Download.SHA384 == "" { + return nil, fmt.Errorf("bin %q has no download digest", name) } - return nil, fmt.Errorf("bin %q has no %s/%s release for architecture %q", info.Name, track, risk, arch) + return rev, nil } // nameExp matches a valid package name. It deliberately forbids "/" and any @@ -224,12 +237,10 @@ func (s *binStore) Options() *Options { } func (s *binStore) Info(name, track, risk string) (*archive.PackageInfo, error) { - risk = resolveRisk(risk) - resp, err := s.fetchBinInfo(name) - if err != nil { - return nil, err + if risk == "" { + risk = defaultRisk } - rev, err := selectRevision(resp, s.options.Arch, track, risk) + rev, err := s.resolveRevision(name, track, risk) if err != nil { return nil, err } @@ -247,14 +258,12 @@ func (s *binStore) Exists(name, track, risk string) bool { } func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive.PackageInfo, error) { - risk = resolveRisk(risk) + if risk == "" { + risk = defaultRisk + } logf("Fetching bin %s %s/%s ...", name, track, risk) - resp, err := s.fetchBinInfo(name) - if err != nil { - return nil, nil, err - } - rev, err := selectRevision(resp, s.options.Arch, track, risk) + rev, err := s.resolveRevision(name, track, risk) if err != nil { return nil, nil, err } @@ -267,7 +276,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. SHA384: rev.Download.SHA384, } - const digestKind = cache.SHA256 + const digestKind = cache.SHA384 // Check cache first. reader, err := s.cache.Open(digestKind, digest) if err == nil { diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 6d29fb167..e162aaef1 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -2,6 +2,7 @@ package store_test import ( "bytes" + "encoding/json" "fmt" "io" "net/http" @@ -73,29 +74,33 @@ func sha384Hash(data []byte) string { return fmt.Sprintf("%x", h.Sum(nil)) } -func makeBinInfoBody(name, track, risk, arch, version string, revision int, sha384 string) []byte { +func makeResolveBody(name, track, risk, arch, version string, revision int, sha384 string) []byte { downloadURL := "https://api.snapcraft.io/api/v1/bins/download/" + name + ".bin" - return makeBinInfoBodyWithURL(name, track, risk, arch, version, revision, sha384, downloadURL) + return makeResolveBodyWithURL(name, track, risk, arch, version, revision, sha384, downloadURL) } -func makeBinInfoBodyWithURL(name, track, risk, arch, version string, revision int, sha384, downloadURL string) []byte { +func makeResolveBodyWithURL(name, track, risk, arch, version string, revision int, sha384, downloadURL string) []byte { return []byte(fmt.Sprintf(`{ - "name": %q, - "channel-map": [ + "craft-results": [], + "package-results": [ { - "channel": { - "name": %q, - "risk": %q, - "track": %q, - "platform": {"architecture": %q} - }, - "revision": { - "version": %q, - "revision": %d, - "download": { - "url": %q, - "sha3-384": %q, - "size": 1024 + "instance-key": %q, + "status": "ok", + "result": { + "channel": { + "name": %q, + "risk": %q, + "track": %q, + "platform": {"architecture": %q} + }, + "revision": { + "version": %q, + "revision": %d, + "download": { + "url": %q, + "sha3-384": %q, + "size": 1024 + } } } } @@ -103,6 +108,20 @@ func makeBinInfoBodyWithURL(name, track, risk, arch, version string, revision in }`, name, track+"/"+risk, risk, track, arch, version, revision, downloadURL, sha384)) } +func makeResolveErrorBody(name, code, message string) []byte { + return []byte(fmt.Sprintf(`{ + "craft-results": [], + "package-results": [ + { + "instance-key": %q, + "status": "error", + "error": {"code": %q, "message": %q}, + "result": null + } + ] + }`, name, code, message)) +} + func (s *storeSuite) TestValidateDownloadURL(c *C) { tests := []struct { url string @@ -193,26 +212,20 @@ var infoTests = []infoTest{{ summary: "Successful info", risk: "stable", status: 200, - body: string(makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123")), + body: string(makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123")), info: &store.StorePackageInfo{Name: "curl", Version: "8.5.0", Revision: 42, SHA384: "abc123"}, }, { summary: "Defaults to stable risk when unspecified", risk: "", status: 200, - body: string(makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123")), + body: string(makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123")), info: &store.StorePackageInfo{Name: "curl", Version: "8.5.0", Revision: 42, SHA384: "abc123"}, }, { summary: "Package not found", risk: "stable", - status: 404, - body: "not found", - error: `bin "curl" not found`, -}, { - summary: "No release for the requested architecture", - risk: "stable", status: 200, - body: string(makeBinInfoBody("curl", "latest", "stable", "arm64", "8.5.0", 42, "abc123")), - error: `bin "curl" has no latest/stable release for architecture "amd64"`, + body: string(makeResolveErrorBody("curl", "package-not-found", "Package not found")), + error: `bin "curl" not found: Package not found`, }, { summary: "Server error", risk: "stable", @@ -226,29 +239,11 @@ var infoTests = []infoTest{{ status: 200, body: "not json", error: "cannot decode bin store response: .*", -}, { - summary: "Selects the entry matching the requested architecture", - risk: "stable", - status: 200, - body: `{ - "name": "curl", - "channel-map": [ - { - "channel": {"name": "latest/stable", "risk": "stable", "track": "latest", "platform": {"architecture": "arm64"}}, - "revision": {"version": "8.0.0", "revision": 1, "download": {"url": "https://api.snapcraft.io/api/v1/bins/download/curl.bin", "sha3-384": "arm64hash", "size": 1024}} - }, - { - "channel": {"name": "latest/stable", "risk": "stable", "track": "latest", "platform": {"architecture": "amd64"}}, - "revision": {"version": "8.5.0", "revision": 42, "download": {"url": "https://api.snapcraft.io/api/v1/bins/download/curl.bin", "sha3-384": "amd64hash", "size": 1024}} - } - ] - }`, - info: &store.StorePackageInfo{Name: "curl", Version: "8.5.0", Revision: 42, SHA384: "amd64hash"}, }, { summary: "Missing download digest", risk: "stable", status: 200, - body: string(makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "")), + body: string(makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "")), error: `bin "curl" has no download digest`, }} @@ -282,11 +277,26 @@ func (s *storeSuite) TestInfo(c *C) { } func (s *storeSuite) TestInfoRequest(c *C) { - infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + infoBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { - c.Assert(req.URL.Path, Equals, "/v2/bins/info/curl") - c.Assert(req.URL.Query().Get("fields"), Equals, "download,version,revision,channel-map") + c.Assert(req.Method, Equals, "POST") + c.Assert(req.URL.Path, Equals, "/v2/revisions/resolve") + c.Assert(req.Header.Get("Content-Type"), Equals, "application/json") + c.Assert(req.Header.Get("Accept"), Equals, "application/json") + + var body map[string]any + err := json.NewDecoder(req.Body).Decode(&body) + c.Assert(err, IsNil) + pkgs := body["packages"].([]any) + c.Assert(pkgs, HasLen, 1) + pkg := pkgs[0].(map[string]any) + c.Assert(pkg["instance-key"], Equals, "curl") + c.Assert(pkg["namespace"], Equals, "bin") + c.Assert(pkg["name"], Equals, "curl") + c.Assert(pkg["channel"], Equals, "latest/stable") + c.Assert(pkg["platform"].(map[string]any)["architecture"], Equals, "amd64") + return &http.Response{ StatusCode: 200, Body: io.NopCloser(bytes.NewReader(infoBody)), @@ -305,18 +315,30 @@ func (s *storeSuite) TestInfoRequest(c *C) { } func (s *storeSuite) TestExists(c *C) { - infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + okBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + notFoundBody := makeResolveErrorBody("nonexistent", "package-not-found", "Package not found") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { - if strings.Contains(req.URL.Path, "/info/curl") { + var body struct { + Packages []struct { + Name string `json:"name"` + } `json:"packages"` + } + err := json.NewDecoder(req.Body).Decode(&body) + c.Assert(err, IsNil) + pkgName := "" + if len(body.Packages) > 0 { + pkgName = body.Packages[0].Name + } + if pkgName == "curl" { return &http.Response{ StatusCode: 200, - Body: io.NopCloser(bytes.NewReader(infoBody)), + Body: io.NopCloser(bytes.NewReader(okBody)), }, nil } return &http.Response{ - StatusCode: 404, - Body: io.NopCloser(strings.NewReader("not found")), + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(notFoundBody)), }, nil } @@ -335,12 +357,12 @@ func (s *storeSuite) TestFetchCacheMiss(c *C) { tarData := []byte("fake tar.xz content") digest := sha384Hash(tarData) - infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) + infoBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) callCount := 0 s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { callCount++ - if strings.Contains(req.URL.Path, "/info/") { + if req.URL.Path == "/v2/revisions/resolve" { return &http.Response{ StatusCode: 200, Body: io.NopCloser(bytes.NewReader(infoBody)), @@ -389,11 +411,11 @@ func (s *storeSuite) TestFetchCacheHit(c *C) { err := cc.Write(cache.SHA384, digest, tarData) c.Assert(err, IsNil) - infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) + infoBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) infoCallCount := 0 s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { - if strings.Contains(req.URL.Path, "/info/") { + if req.URL.Path == "/v2/revisions/resolve" { infoCallCount++ return &http.Response{ StatusCode: 200, @@ -425,7 +447,7 @@ func (s *storeSuite) TestFetchCacheHit(c *C) { func (s *storeSuite) TestFetchInvalidDownloadURL(c *C) { // Override the download URL to an invalid one. - infoBody := makeBinInfoBodyWithURL("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123", + infoBody := makeResolveBodyWithURL("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123", "http://evil.example.com/bins/curl.tar.xz") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { @@ -448,7 +470,7 @@ func (s *storeSuite) TestFetchInvalidDownloadURL(c *C) { func (s *storeSuite) TestFetchMissingDigest(c *C) { // The store omits the sha3-384 digest from the response. - infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "") + infoBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { return &http.Response{ @@ -479,7 +501,7 @@ func (s *storeSuite) TestStagingEnvVar(c *C) { }) c.Assert(err, IsNil) - infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + infoBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { // Verify staging URL is used. @@ -504,10 +526,10 @@ func (s *storeSuite) TestOpenUnsupportedKind(c *C) { } func (s *storeSuite) TestFetchDownloadError(c *C) { - infoBody := makeBinInfoBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + infoBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { - if strings.Contains(req.URL.Path, "/info/") { + if req.URL.Path == "/v2/revisions/resolve" { return &http.Response{ StatusCode: 200, Body: io.NopCloser(bytes.NewReader(infoBody)), From e1fd757476a9ad078bc121e628e40cafcc80f814 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 23 Jun 2026 15:42:57 +0200 Subject: [PATCH 13/20] fix: raise consistency --- cmd/chisel/cmd_cut.go | 4 ++-- internal/slicer/slicer.go | 4 +++- internal/store/export_test.go | 19 +++++++-------- internal/store/store.go | 33 +++++++++++++------------ internal/store/store_test.go | 45 ++++++++++++++++------------------- 5 files changed, 51 insertions(+), 54 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 8cdd11ce6..8e97454ab 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -123,7 +123,7 @@ func (cmd *cmdCut) Execute(args []string) error { } stores := make(map[string]store.Store) - for _, storeInfo := range release.Stores { + for storeName, storeInfo := range release.Stores { openStore, err := store.Open(&store.Options{ Arch: cmd.Arch, CacheDir: cache.DefaultDir("chisel"), @@ -133,7 +133,7 @@ func (cmd *cmdCut) Execute(args []string) error { if err != nil { return err } - stores[storeInfo.Name] = openStore + stores[storeName] = openStore } err = slicer.Run(&slicer.RunOptions{ diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 6e9852731..b71ef6d75 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -541,9 +541,11 @@ func resolvePkgSources(archives map[string]archive.Archive, stores map[string]st // The package metadata returned by the store is not yet recorded // in the manifest; this is handled in a subsequent change. // Risk is left unspecified for now; the store applies its default. + // The store channel track is "-", + // e.g. "3.1-26.10". The version pins the release series. track := pkg.DefaultTrack + "-" + storeHandle.Options().Version pkgSources[pkg.Name] = pkgSource{ - // TODO: set the arch when implementing fetching from the store. + arch: storeHandle.Options().Arch, fetch: func() (io.ReadSeekCloser, *archive.PackageInfo, error) { return storeHandle.Fetch(pkg.RealName, track, "") }, diff --git a/internal/store/export_test.go b/internal/store/export_test.go index f69322be8..cdaf5d1d6 100644 --- a/internal/store/export_test.go +++ b/internal/store/export_test.go @@ -7,14 +7,13 @@ var ( BinStagingEnvVar = binStagingEnvVar ) -func SetHTTPDo(fn func(req *http.Request) (*http.Response, error)) (restore func()) { - saved := httpDo - httpDo = fn - return func() { httpDo = saved } -} - -func SetBulkDo(fn func(req *http.Request) (*http.Response, error)) (restore func()) { - saved := bulkDo - bulkDo = fn - return func() { bulkDo = saved } +func FakeDo(do func(req *http.Request) (*http.Response, error)) (restore func()) { + _httpDo := httpDo + _bulkDo := bulkDo + httpDo = do + bulkDo = do + return func() { + httpDo = _httpDo + bulkDo = _bulkDo + } } diff --git a/internal/store/store.go b/internal/store/store.go index 5630ea1d2..6711a2d20 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -141,7 +141,6 @@ type binRevision struct { type binDownload struct { URL string `json:"url"` SHA384 string `json:"sha3-384"` - Size int64 `json:"size"` } // resolveRevision resolves a single package revision via the store API. It @@ -179,33 +178,33 @@ func (s *binStore) resolveRevision(name, track, risk string) (*binRevision, erro resp, err := httpDo(req) if err != nil { - return nil, fmt.Errorf("cannot talk to bin store: %v", err) + return nil, fmt.Errorf("cannot talk to store: %v", err) } defer resp.Body.Close() if resp.StatusCode != 200 { - return nil, fmt.Errorf("cannot fetch from bin store: %v", resp.Status) + return nil, fmt.Errorf("cannot fetch from store: %v", resp.Status) } var res resolveResponse err = json.NewDecoder(resp.Body).Decode(&res) if err != nil { - return nil, fmt.Errorf("cannot decode bin store response: %v", err) + return nil, fmt.Errorf("cannot decode store response: %v", err) } if len(res.PackageResults) == 0 { - return nil, fmt.Errorf("bin %q not found", name) + return nil, fmt.Errorf("package %q not found", name) } result := &res.PackageResults[0] if result.Status != "ok" || result.Result == nil { if result.Error != nil { - return nil, fmt.Errorf("bin %q not found: %s", name, result.Error.Message) + return nil, fmt.Errorf("package %q not found: %s", name, result.Error.Message) } - return nil, fmt.Errorf("bin %q not found", name) + return nil, fmt.Errorf("package %q not found", name) } rev := &result.Result.Revision if rev.Download.SHA384 == "" { - return nil, fmt.Errorf("bin %q has no download digest", name) + return nil, fmt.Errorf("package %q has no download digest", name) } return rev, nil } @@ -220,16 +219,16 @@ var nameExp = regexp.MustCompile(`^[a-z0-9][a-z0-9+.-]*$`) func validateDownloadURL(downloadURL, allowedHost string) error { u, err := url.Parse(downloadURL) if err != nil { - return fmt.Errorf("cannot parse bin download URL: %v", err) + return fmt.Errorf("cannot parse download URL: %v", err) } if u.Scheme != "https" { - return fmt.Errorf("bin download URL must use HTTPS: %q", downloadURL) + return fmt.Errorf("download URL must use HTTPS: %q", downloadURL) } host := strings.ToLower(u.Hostname()) if host == allowedHost || strings.HasSuffix(host, "."+allowedHost) { return nil } - return fmt.Errorf("bin download URL has untrusted host %q", host) + return fmt.Errorf("download URL has untrusted host %q", host) } func (s *binStore) Options() *Options { @@ -261,7 +260,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. if risk == "" { risk = defaultRisk } - logf("Fetching bin %s %s/%s ...", name, track, risk) + logf("Fetching package %s %s/%s...", name, track, risk) rev, err := s.resolveRevision(name, track, risk) if err != nil { @@ -280,13 +279,13 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. // Check cache first. reader, err := s.cache.Open(digestKind, digest) if err == nil { - logf("Using cached bin %s", name) + logf("Using cached package %s", name) return reader, info, nil } else if err != cache.ErrMiss { return nil, nil, err } - // Download the bin. + // Download the package. err = validateDownloadURL(rev.Download.URL, s.downloadHost) if err != nil { return nil, nil, err @@ -298,12 +297,12 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. httpResp, err := bulkDo(req) if err != nil { - return nil, nil, fmt.Errorf("cannot download bin %q: %v", name, err) + return nil, nil, fmt.Errorf("cannot download package %q: %v", name, err) } defer httpResp.Body.Close() if httpResp.StatusCode != 200 { - return nil, nil, fmt.Errorf("cannot download bin %q: %v", name, httpResp.Status) + return nil, nil, fmt.Errorf("cannot download package %q: %v", name, httpResp.Status) } writer := s.cache.Create(digestKind, digest) @@ -314,7 +313,7 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. err = writer.Close() } if err != nil { - return nil, nil, fmt.Errorf("cannot fetch bin %q: %v", name, err) + return nil, nil, fmt.Errorf("cannot fetch package %q: %v", name, err) } reader, err = s.cache.Open(digestKind, writer.Digest()) diff --git a/internal/store/store_test.go b/internal/store/store_test.go index e162aaef1..381632d49 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -18,12 +18,11 @@ import ( ) type storeSuite struct { - tempDir string - cacheDir string - fakeDoFunc func(req *http.Request) (*http.Response, error) - restoreDo func() - restoreBulk func() - envRestore func() + tempDir string + cacheDir string + fakeDoFunc func(req *http.Request) (*http.Response, error) + restore func() + envRestore func() } var _ = Suite(&storeSuite{}) @@ -34,14 +33,12 @@ func (s *storeSuite) SetUpTest(c *C) { c.Assert(os.MkdirAll(s.cacheDir, 0o755), IsNil) s.envRestore = fakeEnv("") - s.restoreDo = store.SetHTTPDo(s.doRequest) - s.restoreBulk = store.SetBulkDo(s.doRequest) + s.restore = store.FakeDo(s.doRequest) s.fakeDoFunc = nil } func (s *storeSuite) TearDownTest(c *C) { - s.restoreDo() - s.restoreBulk() + s.restore() s.envRestore() } @@ -133,7 +130,7 @@ func (s *storeSuite) TestValidateDownloadURL(c *C) { { "https://api.staging.snapcraft.io/api/v1/bins/download/foo.bin", "api.snapcraft.io", - `bin download URL has untrusted host "api.staging.snapcraft.io"`, + `download URL has untrusted host "api.staging.snapcraft.io"`, }, // Staging host is allowed when it is the allowed host. {"https://api.staging.snapcraft.io/api/v1/bins/download/foo.bin", "api.staging.snapcraft.io", ""}, @@ -141,29 +138,29 @@ func (s *storeSuite) TestValidateDownloadURL(c *C) { { "https://api.snapcraft.io/api/v1/bins/download/foo.bin", "api.staging.snapcraft.io", - `bin download URL has untrusted host "api.snapcraft.io"`, + `download URL has untrusted host "api.snapcraft.io"`, }, { "http://api.snapcraft.io/api/v1/bins/download/foo.bin", "api.snapcraft.io", - `bin download URL must use HTTPS: "http://api.snapcraft.io/api/v1/bins/download/foo.bin"`, + `download URL must use HTTPS: "http://api.snapcraft.io/api/v1/bins/download/foo.bin"`, }, { "https://evil.example.com/api/v1/bins/download/foo.bin", "api.snapcraft.io", - `bin download URL has untrusted host "evil.example.com"`, + `download URL has untrusted host "evil.example.com"`, }, { "https://Evil.Example.Com/api/v1/bins/download/foo.bin", "api.snapcraft.io", - `bin download URL has untrusted host "evil.example.com"`, + `download URL has untrusted host "evil.example.com"`, }, { "https://api.snapcraft.io.evil.com/api/v1/bins/download/foo.bin", "api.snapcraft.io", - `bin download URL has untrusted host "api.snapcraft.io.evil.com"`, + `download URL has untrusted host "api.snapcraft.io.evil.com"`, }, - {"://invalid-url", "api.snapcraft.io", `cannot parse bin download URL: .*`}, + {"://invalid-url", "api.snapcraft.io", `cannot parse download URL: .*`}, } for _, test := range tests { err := store.ValidateDownloadURL(test.url, test.allowedHost) @@ -225,26 +222,26 @@ var infoTests = []infoTest{{ risk: "stable", status: 200, body: string(makeResolveErrorBody("curl", "package-not-found", "Package not found")), - error: `bin "curl" not found: Package not found`, + error: `package "curl" not found: Package not found`, }, { summary: "Server error", risk: "stable", status: 500, statusText: "500 Internal Server Error", body: "boom", - error: "cannot fetch from bin store: 500 Internal Server Error", + error: "cannot fetch from store: 500 Internal Server Error", }, { summary: "Malformed response body", risk: "stable", status: 200, body: "not json", - error: "cannot decode bin store response: .*", + error: "cannot decode store response: .*", }, { summary: "Missing download digest", risk: "stable", status: 200, body: string(makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "")), - error: `bin "curl" has no download digest`, + error: `package "curl" has no download digest`, }} func (s *storeSuite) TestInfo(c *C) { @@ -465,7 +462,7 @@ func (s *storeSuite) TestFetchInvalidDownloadURL(c *C) { c.Assert(err, IsNil) _, _, err = src.Fetch("curl", "latest", "stable") - c.Assert(err, ErrorMatches, `bin download URL must use HTTPS: "http://evil.example.com/bins/curl.tar.xz"`) + c.Assert(err, ErrorMatches, `download URL must use HTTPS: "http://evil.example.com/bins/curl.tar.xz"`) } func (s *storeSuite) TestFetchMissingDigest(c *C) { @@ -487,7 +484,7 @@ func (s *storeSuite) TestFetchMissingDigest(c *C) { c.Assert(err, IsNil) _, _, err = src.Fetch("curl", "latest", "stable") - c.Assert(err, ErrorMatches, `bin "curl" has no download digest`) + c.Assert(err, ErrorMatches, `package "curl" has no download digest`) } func (s *storeSuite) TestStagingEnvVar(c *C) { @@ -550,5 +547,5 @@ func (s *storeSuite) TestFetchDownloadError(c *C) { c.Assert(err, IsNil) _, _, err = src.Fetch("curl", "latest", "stable") - c.Assert(err, ErrorMatches, `cannot download bin "curl": 500 Internal Server Error`) + c.Assert(err, ErrorMatches, `cannot download package "curl": 500 Internal Server Error`) } From 846f440f018cf7d5a4eaa385cbb609765ffe522c Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 23 Jun 2026 15:46:18 +0200 Subject: [PATCH 14/20] fix: use RealName in extract error --- internal/slicer/slicer_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/slicer/slicer_test.go b/internal/slicer/slicer_test.go index c6eab7883..df88fa31a 100644 --- a/internal/slicer/slicer_test.go +++ b/internal/slicer/slicer_test.go @@ -2010,7 +2010,7 @@ var slicerTests = []slicerTest{{ /dir/store-file: `, }, - error: `cannot fetch package "bin-store-pkg" from store "bin": not implemented`, + error: `cannot extract package "store-pkg" from store: store packages are not yet supported`, }} func (s *S) TestRun(c *C) { From 51ac9bc8386c78ad5b98d5a7c3ac75db81e3c50d Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 23 Jun 2026 15:54:23 +0200 Subject: [PATCH 15/20] fix: clean model and add guardrail --- internal/store/store.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/store/store.go b/internal/store/store.go index 6711a2d20..e70e4e260 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -120,7 +120,6 @@ type resolveResult struct { } type resolveError struct { - Code string `json:"code"` Message string `json:"message"` } @@ -195,6 +194,9 @@ func (s *binStore) resolveRevision(name, track, risk string) (*binRevision, erro if len(res.PackageResults) == 0 { return nil, fmt.Errorf("package %q not found", name) } + if len(res.PackageResults) > 1 { + return nil, fmt.Errorf("internal error: store returned %d results for package %q", len(res.PackageResults), name) + } result := &res.PackageResults[0] if result.Status != "ok" || result.Result == nil { if result.Error != nil { From 736dca11ff4b505cd25af79dcb7687b4f873659f Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 23 Jun 2026 17:27:54 +0200 Subject: [PATCH 16/20] refactor: clean useless method --- internal/store/store.go | 19 +++++++++--------- internal/store/store_test.go | 39 ------------------------------------ internal/testutil/store.go | 5 ----- 3 files changed, 9 insertions(+), 54 deletions(-) diff --git a/internal/store/store.go b/internal/store/store.go index e70e4e260..90d770123 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -21,7 +21,6 @@ import ( type Store interface { Options() *Options Fetch(name, track, risk string) (io.ReadSeekCloser, *archive.PackageInfo, error) - Exists(name, track, risk string) bool Info(name, track, risk string) (*archive.PackageInfo, error) } @@ -181,6 +180,10 @@ func (s *binStore) resolveRevision(name, track, risk string) (*binRevision, erro } defer resp.Body.Close() + // The resolve endpoint returns 200 for both success and per-package errors + // (e.g. package not found). Request-level errors (e.g. malformed channel) + // return 4xx. A simple status check is sufficient here because per-package + // errors are handled below via the result status field. if resp.StatusCode != 200 { return nil, fmt.Errorf("cannot fetch from store: %v", resp.Status) } @@ -211,9 +214,10 @@ func (s *binStore) resolveRevision(name, track, risk string) (*binRevision, erro return rev, nil } -// nameExp matches a valid package name. It deliberately forbids "/" and any -// leading "." so that a name cannot be used to traverse or otherwise alter the -// store API URL path when interpolated into it. +// nameExp matches a valid package name. It is used to validate the name +// before sending it to the store API. Unlike other payload values (track, +// risk, arch) which are validated upstream, the package name comes directly +// from the release YAML without format validation. var nameExp = regexp.MustCompile(`^[a-z0-9][a-z0-9+.-]*$`) // validateDownloadURL checks that the download URL is HTTPS and from the @@ -227,7 +231,7 @@ func validateDownloadURL(downloadURL, allowedHost string) error { return fmt.Errorf("download URL must use HTTPS: %q", downloadURL) } host := strings.ToLower(u.Hostname()) - if host == allowedHost || strings.HasSuffix(host, "."+allowedHost) { + if host == allowedHost { return nil } return fmt.Errorf("download URL has untrusted host %q", host) @@ -253,11 +257,6 @@ func (s *binStore) Info(name, track, risk string) (*archive.PackageInfo, error) }, nil } -func (s *binStore) Exists(name, track, risk string) bool { - _, err := s.Info(name, track, risk) - return err == nil -} - func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive.PackageInfo, error) { if risk == "" { risk = defaultRisk diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 381632d49..817287f2b 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -311,45 +311,6 @@ func (s *storeSuite) TestInfoRequest(c *C) { c.Assert(err, IsNil) } -func (s *storeSuite) TestExists(c *C) { - okBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") - notFoundBody := makeResolveErrorBody("nonexistent", "package-not-found", "Package not found") - - s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { - var body struct { - Packages []struct { - Name string `json:"name"` - } `json:"packages"` - } - err := json.NewDecoder(req.Body).Decode(&body) - c.Assert(err, IsNil) - pkgName := "" - if len(body.Packages) > 0 { - pkgName = body.Packages[0].Name - } - if pkgName == "curl" { - return &http.Response{ - StatusCode: 200, - Body: io.NopCloser(bytes.NewReader(okBody)), - }, nil - } - return &http.Response{ - StatusCode: 200, - Body: io.NopCloser(bytes.NewReader(notFoundBody)), - }, nil - } - - src, err := store.Open(&store.Options{ - Arch: "amd64", - CacheDir: s.cacheDir, - Kind: "bin", - }) - c.Assert(err, IsNil) - - c.Assert(src.Exists("curl", "latest", "stable"), Equals, true) - c.Assert(src.Exists("nonexistent", "latest", "stable"), Equals, false) -} - func (s *storeSuite) TestFetchCacheMiss(c *C) { tarData := []byte("fake tar.xz content") digest := sha384Hash(tarData) diff --git a/internal/testutil/store.go b/internal/testutil/store.go index b985a6aa6..4f2ece30c 100644 --- a/internal/testutil/store.go +++ b/internal/testutil/store.go @@ -31,11 +31,6 @@ func (s *TestStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive return ReadSeekNopCloser(bytes.NewReader(pkg.Data)), info, nil } -func (s *TestStore) Exists(name, track, risk string) bool { - _, ok := s.Packages[name] - return ok -} - func (s *TestStore) Info(name, track, risk string) (*archive.PackageInfo, error) { pkg, ok := s.Packages[name] if !ok { From fe06c7320a0890a46ca1ddee08f0d84928fffe43 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Fri, 26 Jun 2026 09:24:20 +0200 Subject: [PATCH 17/20] fix: review --- cmd/chisel/cmd_cut.go | 6 ++ internal/slicer/slicer.go | 9 +- internal/store/store.go | 73 +++++++-------- internal/store/store_test.go | 170 ++++++++++++++++------------------- internal/store/suite_test.go | 9 ++ internal/testutil/store.go | 12 --- 6 files changed, 129 insertions(+), 150 deletions(-) diff --git a/cmd/chisel/cmd_cut.go b/cmd/chisel/cmd_cut.go index 8e97454ab..fd82fcd29 100644 --- a/cmd/chisel/cmd_cut.go +++ b/cmd/chisel/cmd_cut.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "slices" "time" @@ -131,6 +132,11 @@ func (cmd *cmdCut) Execute(args []string) error { Version: storeInfo.Version, }) if err != nil { + var unknownStoreKindError *store.UnknownStoreKindError + if errors.As(err, &unknownStoreKindError) { + logf("Store %q ignored: %v", storeName, err) + continue + } return err } stores[storeName] = openStore diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index b71ef6d75..4ffb50ed0 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -253,8 +253,8 @@ func Run(options *RunOptions) error { // src := pkgSources[slice.Package] // Store packages are distributed as plain tarballs, whose extraction // is not yet implemented. Fail until the format support is in place. - // if src.kind == sourceStore { - // return fmt.Errorf("cannot extract package %q from store: store packages are not yet supported", src.pkg.Name) + // if src.kind != sourceArchive { + // return fmt.Errorf("cannot extract package %q from store: store packages are not yet supported", src.pkg.RealName) // } err := tarball.Extract(reader, &tarball.ExtractOptions{ Package: slice.Package, @@ -538,11 +538,10 @@ func resolvePkgSources(archives map[string]archive.Archive, stores map[string]st if storeHandle == nil { return nil, fmt.Errorf("internal error: no store handle for store %q", pkg.Store) } - // The package metadata returned by the store is not yet recorded - // in the manifest; this is handled in a subsequent change. - // Risk is left unspecified for now; the store applies its default. // The store channel track is "-", // e.g. "3.1-26.10". The version pins the release series. + // Risk is left unspecified for now; the store applies its default. + // In the future the risk will optionnaly come from the CLI. track := pkg.DefaultTrack + "-" + storeHandle.Options().Version pkgSources[pkg.Name] = pkgSource{ arch: storeHandle.Options().Arch, diff --git a/internal/store/store.go b/internal/store/store.go index 90d770123..9611651ae 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -21,7 +21,6 @@ import ( type Store interface { Options() *Options Fetch(name, track, risk string) (io.ReadSeekCloser, *archive.PackageInfo, error) - Info(name, track, risk string) (*archive.PackageInfo, error) } type Options struct { @@ -64,6 +63,20 @@ var bulkClient = &http.Client{ var bulkDo = bulkClient.Do +// nameExp matches a valid package name. It is used to validate the name +// before sending it to the store API. Unlike other payload values (track, +// risk, arch) which are validated upstream, the package name comes directly +// from the release YAML without format validation. +var nameExp = regexp.MustCompile(`^[a-z0-9][a-z0-9+.-]*$`) + +type UnknownStoreKindError struct { + kind string +} + +func (e *UnknownStoreKindError) Error() string { + return fmt.Sprintf("unsupported store kind %q", e.kind) +} + func Open(options *Options) (Store, error) { var err error if options.Arch == "" { @@ -90,7 +103,7 @@ func Open(options *Options) (Store, error) { downloadHost: downloadHost, }, nil default: - return nil, fmt.Errorf("unsupported store kind %q", options.Kind) + return nil, &UnknownStoreKindError{kind: options.Kind} } } @@ -214,49 +227,10 @@ func (s *binStore) resolveRevision(name, track, risk string) (*binRevision, erro return rev, nil } -// nameExp matches a valid package name. It is used to validate the name -// before sending it to the store API. Unlike other payload values (track, -// risk, arch) which are validated upstream, the package name comes directly -// from the release YAML without format validation. -var nameExp = regexp.MustCompile(`^[a-z0-9][a-z0-9+.-]*$`) - -// validateDownloadURL checks that the download URL is HTTPS and from the -// allowed host. -func validateDownloadURL(downloadURL, allowedHost string) error { - u, err := url.Parse(downloadURL) - if err != nil { - return fmt.Errorf("cannot parse download URL: %v", err) - } - if u.Scheme != "https" { - return fmt.Errorf("download URL must use HTTPS: %q", downloadURL) - } - host := strings.ToLower(u.Hostname()) - if host == allowedHost { - return nil - } - return fmt.Errorf("download URL has untrusted host %q", host) -} - func (s *binStore) Options() *Options { return &s.options } -func (s *binStore) Info(name, track, risk string) (*archive.PackageInfo, error) { - if risk == "" { - risk = defaultRisk - } - rev, err := s.resolveRevision(name, track, risk) - if err != nil { - return nil, err - } - return &archive.PackageInfo{ - Name: name, - Version: rev.Version, - Revision: rev.Revision, - SHA384: rev.Download.SHA384, - }, nil -} - func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive.PackageInfo, error) { if risk == "" { risk = defaultRisk @@ -323,3 +297,20 @@ func (s *binStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive. } return reader, info, nil } + +// validateDownloadURL checks that the download URL is HTTPS and from the +// allowed host. +func validateDownloadURL(downloadURL, allowedHost string) error { + u, err := url.Parse(downloadURL) + if err != nil { + return fmt.Errorf("cannot parse download URL: %v", err) + } + if u.Scheme != "https" { + return fmt.Errorf("download URL must use HTTPS: %q", downloadURL) + } + host := strings.ToLower(u.Hostname()) + if host == allowedHost { + return nil + } + return fmt.Errorf("download URL has untrusted host %q", host) +} diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 817287f2b..ca9f8805b 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -32,7 +32,7 @@ func (s *storeSuite) SetUpTest(c *C) { s.cacheDir = filepath.Join(s.tempDir, "cache") c.Assert(os.MkdirAll(s.cacheDir, 0o755), IsNil) - s.envRestore = fakeEnv("") + s.envRestore = fakeEnv(store.BinStagingEnvVar, "") s.restore = store.FakeDo(s.doRequest) s.fakeDoFunc = nil } @@ -49,18 +49,14 @@ func (s *storeSuite) doRequest(req *http.Request) (*http.Response, error) { return nil, fmt.Errorf("unexpected HTTP request: %s", req.URL.String()) } -func fakeEnv(staging string) func() { - oldStaging := os.Getenv(store.BinStagingEnvVar) - if staging != "" { - os.Setenv(store.BinStagingEnvVar, staging) - } else { - os.Unsetenv(store.BinStagingEnvVar) - } +func fakeEnv(name, value string) (restore func()) { + origValue, origSet := os.LookupEnv(name) + os.Setenv(name, value) return func() { - if oldStaging != "" { - os.Setenv(store.BinStagingEnvVar, oldStaging) + if origSet { + os.Setenv(name, origValue) } else { - os.Unsetenv(store.BinStagingEnvVar) + os.Unsetenv(name) } } } @@ -121,48 +117,40 @@ func makeResolveErrorBody(name, code, message string) []byte { func (s *storeSuite) TestValidateDownloadURL(c *C) { tests := []struct { + summary string url string allowedHost string error string }{ - {"https://api.snapcraft.io/api/v1/bins/download/foo.bin", "api.snapcraft.io", ""}, - // Staging host is rejected when the production host is allowed. - { - "https://api.staging.snapcraft.io/api/v1/bins/download/foo.bin", - "api.snapcraft.io", - `download URL has untrusted host "api.staging.snapcraft.io"`, - }, - // Staging host is allowed when it is the allowed host. - {"https://api.staging.snapcraft.io/api/v1/bins/download/foo.bin", "api.staging.snapcraft.io", ""}, - // Production API host is rejected when the staging host is allowed. - { - "https://api.snapcraft.io/api/v1/bins/download/foo.bin", - "api.staging.snapcraft.io", - `download URL has untrusted host "api.snapcraft.io"`, - }, + {"Valid production host", "https://api.snapcraft.io/api/v1/bins/download/foo.bin", "api.snapcraft.io", ""}, { + "HTTP rejected", "http://api.snapcraft.io/api/v1/bins/download/foo.bin", "api.snapcraft.io", `download URL must use HTTPS: "http://api.snapcraft.io/api/v1/bins/download/foo.bin"`, }, { + "Untrusted host", "https://evil.example.com/api/v1/bins/download/foo.bin", "api.snapcraft.io", `download URL has untrusted host "evil.example.com"`, }, { + "Untrusted host case-insensitive", "https://Evil.Example.Com/api/v1/bins/download/foo.bin", "api.snapcraft.io", `download URL has untrusted host "evil.example.com"`, }, { + "Host suffix attack", "https://api.snapcraft.io.evil.com/api/v1/bins/download/foo.bin", "api.snapcraft.io", `download URL has untrusted host "api.snapcraft.io.evil.com"`, }, - {"://invalid-url", "api.snapcraft.io", `cannot parse download URL: .*`}, + {"Invalid URL", "://invalid-url", "api.snapcraft.io", `cannot parse download URL: .*`}, } for _, test := range tests { + c.Logf("Summary: %s", test.summary) err := store.ValidateDownloadURL(test.url, test.allowedHost) if test.error == "" { c.Assert(err, IsNil) @@ -174,14 +162,16 @@ func (s *storeSuite) TestValidateDownloadURL(c *C) { func (s *storeSuite) TestOpenArchValidation(c *C) { tests := []struct { - arch string - error string + summary string + arch string + error string }{ - {"amd64", ""}, - {"arm64", ""}, - {"invalid", "invalid package architecture: invalid"}, + {"Valid amd64", "amd64", ""}, + {"Valid arm64", "arm64", ""}, + {"Invalid architecture", "invalid", "invalid package architecture: invalid"}, } for _, test := range tests { + c.Logf("Summary: %s", test.summary) _, err := store.Open(&store.Options{ Arch: test.arch, CacheDir: s.cacheDir, @@ -195,28 +185,22 @@ func (s *storeSuite) TestOpenArchValidation(c *C) { } } -type infoTest struct { +type fetchTest struct { summary string risk string status int statusText string body string - info *store.StorePackageInfo error string } -var infoTests = []infoTest{{ - summary: "Successful info", - risk: "stable", - status: 200, - body: string(makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123")), - info: &store.StorePackageInfo{Name: "curl", Version: "8.5.0", Revision: 42, SHA384: "abc123"}, -}, { +var fetchTests = []fetchTest{{ summary: "Defaults to stable risk when unspecified", risk: "", status: 200, - body: string(makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123")), - info: &store.StorePackageInfo{Name: "curl", Version: "8.5.0", Revision: 42, SHA384: "abc123"}, + // Uses a real digest so the cache verification passes. + body: string(makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, + sha384Hash([]byte("fake tar.xz content")))), }, { summary: "Package not found", risk: "stable", @@ -244,15 +228,23 @@ var infoTests = []infoTest{{ error: `package "curl" has no download digest`, }} -func (s *storeSuite) TestInfo(c *C) { - for _, test := range infoTests { +func (s *storeSuite) TestFetch(c *C) { + tarData := []byte("fake tar.xz content") + for _, test := range fetchTests { c.Logf("Summary: %s", test.summary) s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + if req.URL.Path == "/v2/revisions/resolve" { + return &http.Response{ + StatusCode: test.status, + Status: test.statusText, + Body: io.NopCloser(strings.NewReader(test.body)), + }, nil + } + // Download URL. return &http.Response{ - StatusCode: test.status, - Status: test.statusText, - Body: io.NopCloser(strings.NewReader(test.body)), + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(tarData)), }, nil } @@ -263,22 +255,29 @@ func (s *storeSuite) TestInfo(c *C) { }) c.Assert(err, IsNil) - info, err := src.Info("curl", "latest", test.risk) + _, _, err = src.Fetch("curl", "latest", test.risk) if test.error != "" { c.Assert(err, ErrorMatches, test.error) - continue + } else { + c.Assert(err, IsNil) } - c.Assert(err, IsNil) - c.Assert(info, DeepEquals, test.info) } } -func (s *storeSuite) TestInfoRequest(c *C) { - infoBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") +func (s *storeSuite) TestResolveRequest(c *C) { + tarData := []byte("fake tar.xz content") + digest := sha384Hash(tarData) + resolveBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { + if req.URL.Path != "/v2/revisions/resolve" { + // Download request. + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(tarData)), + }, nil + } c.Assert(req.Method, Equals, "POST") - c.Assert(req.URL.Path, Equals, "/v2/revisions/resolve") c.Assert(req.Header.Get("Content-Type"), Equals, "application/json") c.Assert(req.Header.Get("Accept"), Equals, "application/json") @@ -296,7 +295,7 @@ func (s *storeSuite) TestInfoRequest(c *C) { return &http.Response{ StatusCode: 200, - Body: io.NopCloser(bytes.NewReader(infoBody)), + Body: io.NopCloser(bytes.NewReader(resolveBody)), }, nil } @@ -307,7 +306,7 @@ func (s *storeSuite) TestInfoRequest(c *C) { }) c.Assert(err, IsNil) - _, err = src.Info("curl", "latest", "stable") + _, _, err = src.Fetch("curl", "latest", "stable") c.Assert(err, IsNil) } @@ -315,7 +314,7 @@ func (s *storeSuite) TestFetchCacheMiss(c *C) { tarData := []byte("fake tar.xz content") digest := sha384Hash(tarData) - infoBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) + resolveBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) callCount := 0 s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { @@ -323,10 +322,10 @@ func (s *storeSuite) TestFetchCacheMiss(c *C) { if req.URL.Path == "/v2/revisions/resolve" { return &http.Response{ StatusCode: 200, - Body: io.NopCloser(bytes.NewReader(infoBody)), + Body: io.NopCloser(bytes.NewReader(resolveBody)), }, nil } - // Download URL + // Download URL. return &http.Response{ StatusCode: 200, Body: io.NopCloser(bytes.NewReader(tarData)), @@ -369,7 +368,7 @@ func (s *storeSuite) TestFetchCacheHit(c *C) { err := cc.Write(cache.SHA384, digest, tarData) c.Assert(err, IsNil) - infoBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) + resolveBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) infoCallCount := 0 s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { @@ -377,7 +376,7 @@ func (s *storeSuite) TestFetchCacheHit(c *C) { infoCallCount++ return &http.Response{ StatusCode: 200, - Body: io.NopCloser(bytes.NewReader(infoBody)), + Body: io.NopCloser(bytes.NewReader(resolveBody)), }, nil } return nil, fmt.Errorf("download should not be called for cache hit") @@ -405,13 +404,13 @@ func (s *storeSuite) TestFetchCacheHit(c *C) { func (s *storeSuite) TestFetchInvalidDownloadURL(c *C) { // Override the download URL to an invalid one. - infoBody := makeResolveBodyWithURL("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123", + resolveBody := makeResolveBodyWithURL("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123", "http://evil.example.com/bins/curl.tar.xz") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { return &http.Response{ StatusCode: 200, - Body: io.NopCloser(bytes.NewReader(infoBody)), + Body: io.NopCloser(bytes.NewReader(resolveBody)), }, nil } @@ -426,31 +425,9 @@ func (s *storeSuite) TestFetchInvalidDownloadURL(c *C) { c.Assert(err, ErrorMatches, `download URL must use HTTPS: "http://evil.example.com/bins/curl.tar.xz"`) } -func (s *storeSuite) TestFetchMissingDigest(c *C) { - // The store omits the sha3-384 digest from the response. - infoBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "") - - s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { - return &http.Response{ - StatusCode: 200, - Body: io.NopCloser(bytes.NewReader(infoBody)), - }, nil - } - - src, err := store.Open(&store.Options{ - Arch: "amd64", - CacheDir: s.cacheDir, - Kind: "bin", - }) - c.Assert(err, IsNil) - - _, _, err = src.Fetch("curl", "latest", "stable") - c.Assert(err, ErrorMatches, `package "curl" has no download digest`) -} - func (s *storeSuite) TestStagingEnvVar(c *C) { s.envRestore() - s.envRestore = fakeEnv("1") + s.envRestore = fakeEnv(store.BinStagingEnvVar, "1") src, err := store.Open(&store.Options{ Arch: "amd64", @@ -459,18 +436,27 @@ func (s *storeSuite) TestStagingEnvVar(c *C) { }) c.Assert(err, IsNil) - infoBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + tarData := []byte("fake tar.xz content") + digest := sha384Hash(tarData) + resolveBody := makeResolveBodyWithURL("curl", "latest", "stable", "amd64", "8.5.0", 42, digest, + "https://api.staging.snapcraft.io/api/v1/bins/download/curl.bin") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { // Verify staging URL is used. c.Assert(req.URL.Host, Equals, "api.staging.snapcraft.io") + if req.URL.Path == "/v2/revisions/resolve" { + return &http.Response{ + StatusCode: 200, + Body: io.NopCloser(bytes.NewReader(resolveBody)), + }, nil + } return &http.Response{ StatusCode: 200, - Body: io.NopCloser(bytes.NewReader(infoBody)), + Body: io.NopCloser(bytes.NewReader(tarData)), }, nil } - _, err = src.Info("curl", "latest", "stable") + _, _, err = src.Fetch("curl", "latest", "stable") c.Assert(err, IsNil) } @@ -484,13 +470,13 @@ func (s *storeSuite) TestOpenUnsupportedKind(c *C) { } func (s *storeSuite) TestFetchDownloadError(c *C) { - infoBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + resolveBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { if req.URL.Path == "/v2/revisions/resolve" { return &http.Response{ StatusCode: 200, - Body: io.NopCloser(bytes.NewReader(infoBody)), + Body: io.NopCloser(bytes.NewReader(resolveBody)), }, nil } return &http.Response{ diff --git a/internal/store/suite_test.go b/internal/store/suite_test.go index 552ab1cc8..4eb91ecb0 100644 --- a/internal/store/suite_test.go +++ b/internal/store/suite_test.go @@ -3,6 +3,7 @@ package store_test import ( "testing" + "github.com/canonical/chisel/internal/store" . "gopkg.in/check.v1" ) @@ -11,3 +12,11 @@ func Test(t *testing.T) { TestingT(t) } type S struct{} var _ = Suite(&S{}) + +func (s *S) SetUpTest(c *C) { + store.SetLogger(c) +} + +func (s *S) TearDownTest(c *C) { + store.SetLogger(nil) +} diff --git a/internal/testutil/store.go b/internal/testutil/store.go index 4f2ece30c..44f4fedce 100644 --- a/internal/testutil/store.go +++ b/internal/testutil/store.go @@ -30,15 +30,3 @@ func (s *TestStore) Fetch(name, track, risk string) (io.ReadSeekCloser, *archive } return ReadSeekNopCloser(bytes.NewReader(pkg.Data)), info, nil } - -func (s *TestStore) Info(name, track, risk string) (*archive.PackageInfo, error) { - pkg, ok := s.Packages[name] - if !ok { - return nil, fmt.Errorf("cannot find package %q in store", name) - } - return &archive.PackageInfo{ - Name: pkg.Name, - Version: pkg.Version, - SHA384: pkg.Hash, - }, nil -} From 8c03288af4a91f5e373dbaaf726dbd80dde6e60c Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 30 Jun 2026 11:40:29 +0200 Subject: [PATCH 18/20] test: fix inconsistencies --- internal/store/store_test.go | 36 +++++++++++++++--------------------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/internal/store/store_test.go b/internal/store/store_test.go index ca9f8805b..c2c0c7cc7 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -160,23 +160,26 @@ func (s *storeSuite) TestValidateDownloadURL(c *C) { } } -func (s *storeSuite) TestOpenArchValidation(c *C) { +func (s *storeSuite) TestOpenOptionErrors(c *C) { tests := []struct { summary string - arch string + options store.Options error string - }{ - {"Valid amd64", "amd64", ""}, - {"Valid arm64", "arm64", ""}, - {"Invalid architecture", "invalid", "invalid package architecture: invalid"}, - } + }{{ + summary: "Valid amd64", + options: store.Options{Arch: "amd64", CacheDir: s.cacheDir, Kind: "bin"}, + }, { + summary: "Invalid architecture", + options: store.Options{Arch: "invalid", CacheDir: s.cacheDir, Kind: "bin"}, + error: "invalid package architecture: invalid", + }, { + summary: "Unsupported store kind", + options: store.Options{Arch: "amd64", CacheDir: s.cacheDir, Kind: "snap"}, + error: `unsupported store kind "snap"`, + }} for _, test := range tests { c.Logf("Summary: %s", test.summary) - _, err := store.Open(&store.Options{ - Arch: test.arch, - CacheDir: s.cacheDir, - Kind: "bin", - }) + _, err := store.Open(&test.options) if test.error == "" { c.Assert(err, IsNil) } else { @@ -460,15 +463,6 @@ func (s *storeSuite) TestStagingEnvVar(c *C) { c.Assert(err, IsNil) } -func (s *storeSuite) TestOpenUnsupportedKind(c *C) { - _, err := store.Open(&store.Options{ - Arch: "amd64", - CacheDir: s.cacheDir, - Kind: "snap", - }) - c.Assert(err, ErrorMatches, `unsupported store kind "snap"`) -} - func (s *storeSuite) TestFetchDownloadError(c *C) { resolveBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") From bd877a6ed585e040cac37cd74fa2381fbc8d5ed2 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Tue, 30 Jun 2026 13:59:16 +0200 Subject: [PATCH 19/20] test: refine --- internal/store/store_test.go | 75 ++++++++++++++++++++++++++---------- internal/store/suite_test.go | 2 + 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/internal/store/store_test.go b/internal/store/store_test.go index c2c0c7cc7..76d016110 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -67,12 +67,21 @@ func sha384Hash(data []byte) string { return fmt.Sprintf("%x", h.Sum(nil)) } -func makeResolveBody(name, track, risk, arch, version string, revision int, sha384 string) []byte { - downloadURL := "https://api.snapcraft.io/api/v1/bins/download/" + name + ".bin" - return makeResolveBodyWithURL(name, track, risk, arch, version, revision, sha384, downloadURL) +type resolveBodyOptions struct { + name string + track string + risk string + arch string + version string + revision int + sha384 string + downloadURL string } -func makeResolveBodyWithURL(name, track, risk, arch, version string, revision int, sha384, downloadURL string) []byte { +func makeResolveBody(opts resolveBodyOptions) []byte { + if opts.downloadURL == "" { + opts.downloadURL = "https://api.snapcraft.io/api/v1/bins/download/" + opts.name + ".bin" + } return []byte(fmt.Sprintf(`{ "craft-results": [], "package-results": [ @@ -98,7 +107,7 @@ func makeResolveBodyWithURL(name, track, risk, arch, version string, revision in } } ] - }`, name, track+"/"+risk, risk, track, arch, version, revision, downloadURL, sha384)) + }`, opts.name, opts.track+"/"+opts.risk, opts.risk, opts.track, opts.arch, opts.version, opts.revision, opts.downloadURL, opts.sha384)) } func makeResolveErrorBody(name, code, message string) []byte { @@ -202,8 +211,11 @@ var fetchTests = []fetchTest{{ risk: "", status: 200, // Uses a real digest so the cache verification passes. - body: string(makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, - sha384Hash([]byte("fake tar.xz content")))), + body: string(makeResolveBody(resolveBodyOptions{ + name: "curl", track: "latest", risk: "stable", arch: "amd64", + version: "8.5.0", revision: 42, + sha384: sha384Hash([]byte("fake tar.xz content")), + })), }, { summary: "Package not found", risk: "stable", @@ -227,8 +239,11 @@ var fetchTests = []fetchTest{{ summary: "Missing download digest", risk: "stable", status: 200, - body: string(makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "")), - error: `package "curl" has no download digest`, + body: string(makeResolveBody(resolveBodyOptions{ + name: "curl", track: "latest", risk: "stable", arch: "amd64", + version: "8.5.0", revision: 42, + })), + error: `package "curl" has no download digest`, }} func (s *storeSuite) TestFetch(c *C) { @@ -270,7 +285,10 @@ func (s *storeSuite) TestFetch(c *C) { func (s *storeSuite) TestResolveRequest(c *C) { tarData := []byte("fake tar.xz content") digest := sha384Hash(tarData) - resolveBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) + resolveBody := makeResolveBody(resolveBodyOptions{ + name: "curl", track: "latest", risk: "stable", arch: "amd64", + version: "8.5.0", revision: 42, sha384: digest, + }) s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { if req.URL.Path != "/v2/revisions/resolve" { @@ -317,7 +335,10 @@ func (s *storeSuite) TestFetchCacheMiss(c *C) { tarData := []byte("fake tar.xz content") digest := sha384Hash(tarData) - resolveBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) + resolveBody := makeResolveBody(resolveBodyOptions{ + name: "curl", track: "latest", risk: "stable", arch: "amd64", + version: "8.5.0", revision: 42, sha384: digest, + }) callCount := 0 s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { @@ -371,12 +392,15 @@ func (s *storeSuite) TestFetchCacheHit(c *C) { err := cc.Write(cache.SHA384, digest, tarData) c.Assert(err, IsNil) - resolveBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, digest) + resolveBody := makeResolveBody(resolveBodyOptions{ + name: "curl", track: "latest", risk: "stable", arch: "amd64", + version: "8.5.0", revision: 42, sha384: digest, + }) - infoCallCount := 0 + resolveCallCount := 0 s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { if req.URL.Path == "/v2/revisions/resolve" { - infoCallCount++ + resolveCallCount++ return &http.Response{ StatusCode: 200, Body: io.NopCloser(bytes.NewReader(resolveBody)), @@ -398,7 +422,7 @@ func (s *storeSuite) TestFetchCacheHit(c *C) { c.Assert(info.Name, Equals, "curl") c.Assert(info.SHA384, Equals, digest) - c.Assert(infoCallCount, Equals, 1) + c.Assert(resolveCallCount, Equals, 1) data, err := io.ReadAll(reader) c.Assert(err, IsNil) @@ -407,8 +431,12 @@ func (s *storeSuite) TestFetchCacheHit(c *C) { func (s *storeSuite) TestFetchInvalidDownloadURL(c *C) { // Override the download URL to an invalid one. - resolveBody := makeResolveBodyWithURL("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123", - "http://evil.example.com/bins/curl.tar.xz") + resolveBody := makeResolveBody(resolveBodyOptions{ + name: "curl", track: "latest", risk: "stable", arch: "amd64", + version: "8.5.0", revision: 42, + sha384: sha384Hash([]byte("fake tar.xz content")), + downloadURL: "http://evil.example.com/bins/curl.tar.xz", + }) s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { return &http.Response{ @@ -441,8 +469,11 @@ func (s *storeSuite) TestStagingEnvVar(c *C) { tarData := []byte("fake tar.xz content") digest := sha384Hash(tarData) - resolveBody := makeResolveBodyWithURL("curl", "latest", "stable", "amd64", "8.5.0", 42, digest, - "https://api.staging.snapcraft.io/api/v1/bins/download/curl.bin") + resolveBody := makeResolveBody(resolveBodyOptions{ + name: "curl", track: "latest", risk: "stable", arch: "amd64", + version: "8.5.0", revision: 42, sha384: digest, + downloadURL: "https://api.staging.snapcraft.io/api/v1/bins/download/curl.bin", + }) s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { // Verify staging URL is used. @@ -464,7 +495,11 @@ func (s *storeSuite) TestStagingEnvVar(c *C) { } func (s *storeSuite) TestFetchDownloadError(c *C) { - resolveBody := makeResolveBody("curl", "latest", "stable", "amd64", "8.5.0", 42, "abc123") + resolveBody := makeResolveBody(resolveBodyOptions{ + name: "curl", track: "latest", risk: "stable", arch: "amd64", + version: "8.5.0", revision: 42, + sha384: sha384Hash([]byte("fake tar.xz content")), + }) s.fakeDoFunc = func(req *http.Request) (*http.Response, error) { if req.URL.Path == "/v2/revisions/resolve" { diff --git a/internal/store/suite_test.go b/internal/store/suite_test.go index 4eb91ecb0..2eac508f2 100644 --- a/internal/store/suite_test.go +++ b/internal/store/suite_test.go @@ -14,9 +14,11 @@ type S struct{} var _ = Suite(&S{}) func (s *S) SetUpTest(c *C) { + store.SetDebug(true) store.SetLogger(c) } func (s *S) TearDownTest(c *C) { + store.SetDebug(false) store.SetLogger(nil) } From 7e1212d62e947f9a609e804720985be57bd2f766 Mon Sep 17 00:00:00 2001 From: Paul Mars Date: Wed, 15 Jul 2026 16:16:01 +0200 Subject: [PATCH 20/20] fix: revert unwanted changes --- internal/slicer/slicer.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/internal/slicer/slicer.go b/internal/slicer/slicer.go index 4ffb50ed0..0982e0804 100644 --- a/internal/slicer/slicer.go +++ b/internal/slicer/slicer.go @@ -368,9 +368,7 @@ func Run(options *RunOptions) error { return generateManifests(targetDir, options.Selection, report, pkgInfos) } -func generateManifests(targetDir string, selection *setup.Selection, - report *manifestutil.Report, pkgInfos []*archive.PackageInfo, -) error { +func generateManifests(targetDir string, selection *setup.Selection, report *manifestutil.Report, pkgInfos []*archive.PackageInfo) error { manifestSlices := manifestutil.FindPaths(selection.Slices) if len(manifestSlices) == 0 { // Nothing to do. @@ -475,9 +473,9 @@ func createFile(targetDir, relPath string, pathInfo setup.PathInfo) (*fsutil.Ent targetMode := pathInfo.Mode if targetMode == 0 { if pathInfo.Kind == setup.DirPath { - targetMode = 0o755 + targetMode = 0755 } else { - targetMode = 0o644 + targetMode = 0644 } }