diff --git a/docs/docs/changelog.md b/docs/docs/changelog.md index 30dd5c1c..2dbc001d 100644 --- a/docs/docs/changelog.md +++ b/docs/docs/changelog.md @@ -5,6 +5,7 @@ title: Changelog ## [Unreleased](https://github.com/lets-cli/lets/releases/tag/v0.0.X) +* `[Added]` Remote config support: `lets -c https://url` downloads and caches config to `~/.config/lets/remote-configs/`. Use `--no-cache` to force re-download. Only standalone configs (no `mixins:`) are supported for now. * `[Added]` Add `lets self skills` commands to show, install, and update the bundled lets agent skill. * `[Docs]` Document the bundled lets Agent Skill and link it from the config reference. * `[Changed]` Expand the bundled lets agent skill with config authoring guidance. diff --git a/docs/specs/2026-06-13-remote-config-design.md b/docs/specs/2026-06-13-remote-config-design.md new file mode 100644 index 00000000..7465b168 --- /dev/null +++ b/docs/specs/2026-06-13-remote-config-design.md @@ -0,0 +1,87 @@ +--- +name: remote-config-design +description: Design for lets -c https://... remote config fetching with caching (issue #351) +--- + +# Remote Config Design + +**Issue:** [#351](https://github.com/lets-cli/lets/issues/351) + +## Summary + +Allow `lets -c https://url` to download a remote `lets.yaml`, cache it locally, and run commands from it with the invocation directory as the working directory. Add `--no-cache` flag to force re-download. + +## Architecture + +### New flag: `--no-cache` + +Added to `initRootFlags` in `internal/cmd/root.go` and parsed in `parseRootFlags` in `internal/cli/cli.go`, following the same pattern as `--debug` and `--init`. + +### URL detection in `cli.go` + +After parsing root flags, if `rootFlags.config` starts with `http://` or `https://`, call `config.LoadRemote(url, noCache, version)` instead of `config.Load(...)`. The existing `Load` path is untouched. + +### New `internal/fetch` package + +Extract HTTP download + content-type validation from `RemoteMixin.download()` into `internal/fetch/fetch.go`: + +```go +func Download(ctx context.Context, url string) ([]byte, error) +``` + +Same 15-minute timeout and content-type whitelist (`text/plain`, `text/yaml`, `text/x-yaml`, `application/yaml`, `application/x-yaml`). `RemoteMixin` becomes a thin wrapper calling `fetch.Download`. This eliminates duplication. + +### New `LoadRemote` in `internal/config/load.go` + +```go +func LoadRemote(url string, noCache bool, version string) (*config.Config, error) +``` + +- Cache path: `util.LetsUserDir()` + `remote-configs/.yaml` + - Resolves to `~/.config/lets/remote-configs/.yaml` +- Creates `~/.config/lets/remote-configs/` if needed +- Working directory: `os.Getwd()` (CWD at invocation time), passed as `configDir` to `Load` + +## Data Flow + +### Normal flow (no `--no-cache`) + +1. Cache file exists → load directly from cached path, skip HTTP +2. Cache missing → download via `fetch.Download` → persist → load from cached path + +### `--no-cache` flow + +1. Attempt download → persist (overwriting cache) → load +2. Download fails + cache exists → `log.Warnf("failed to refresh remote config, using cached version: %s", err)` → load from cache +3. Download fails + no cache → return error + +### Working directory + +`LoadRemote` calls `Load(cachedPath, cwd, version)` where `cwd = os.Getwd()`. Commands in the remote config execute in the invocation directory unless they specify `work_dir`. + +## Files Changed + +| File | Change | +|------|--------| +| `internal/fetch/fetch.go` | New — extracted `Download` func | +| `internal/fetch/fetch_test.go` | New — unit tests for fetch | +| `internal/config/config/mixin.go` | Refactor `download()` to use `fetch.Download` | +| `internal/config/load.go` | Add `LoadRemote` | +| `internal/config/load_test.go` | Add `LoadRemote` tests | +| `internal/cli/cli.go` | URL detection, call `LoadRemote`, thread `noCache` | +| `internal/cmd/root.go` | Add `--no-cache` flag | + +## Testing + +### `internal/fetch/fetch_test.go` +- Valid YAML content-type → success +- Unsupported content-type → error +- 404 → error +- Non-2xx → error + +### `internal/config/load_test.go` +- `LoadRemote` with `httptest.NewServer` serving valid YAML → loads, caches, runs from CWD +- Cache hit: serve once, call twice, assert server receives only one request +- `--no-cache` refresh: serve two different configs, assert second call gets new content +- `--no-cache` + server down + cache exists: assert warning logged + falls back to cache +- `--no-cache` + server down + no cache: assert error returned diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 4d680cee..4d279d7f 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -12,7 +12,8 @@ import ( "github.com/lets-cli/fang" "github.com/lets-cli/lets/internal/cmd" - "github.com/lets-cli/lets/internal/config" + "github.com/lets-cli/lets/internal/config/config" + loader "github.com/lets-cli/lets/internal/config" "github.com/lets-cli/lets/internal/env" "github.com/lets-cli/lets/internal/logging" "github.com/lets-cli/lets/internal/set" @@ -77,7 +78,16 @@ func Main(version string, buildDate string) int { rootFlags.config = os.Getenv("LETS_CONFIG") } - cfg, err := config.Load(rootFlags.config, configDir, version) + var cfg *config.Config + if isRemoteURL(rootFlags.config) { + if configDir != "" { + log.Warnf("LETS_CONFIG_DIR is ignored when using a remote config URL") + } + + cfg, err = loader.LoadRemote(ctx, rootFlags.config, rootFlags.noCache, version) + } else { + cfg, err = loader.Load(rootFlags.config, configDir, version) + } if err != nil { if failOnConfigError(rootCmd, command, rootFlags) { log.Errorf("config error: %s", err) @@ -270,6 +280,10 @@ func isInteractiveStderr() bool { return isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd) } +func isRemoteURL(s string) bool { + return strings.HasPrefix(s, "http://") || strings.HasPrefix(s, "https://") +} + type flags struct { config string debug int @@ -277,6 +291,7 @@ type flags struct { version bool all bool init bool + noCache bool } // We can not parse --config and --debug flags using cobra.Command.ParseFlags @@ -326,7 +341,7 @@ func parseRootFlags(args []string) (*flags, error) { } f.config = value - } else if len(args[idx:]) > 0 { + } else if idx+1 < len(args) { f.config = args[idx+1] idx += 2 @@ -356,6 +371,10 @@ func parseRootFlags(args []string) (*flags, error) { if !isFlagVisited("init") { f.init = true } + case "--no-cache": + if !isFlagVisited("no-cache") { + f.noCache = true + } case "--upgrade": return nil, errors.New("--upgrade has been replaced with 'lets self upgrade'") } diff --git a/internal/cmd/root.go b/internal/cmd/root.go index c6a2f7c2..cfa89cca 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -108,4 +108,5 @@ func initRootFlags(rootCmd *cobra.Command) { rootCmd.Flags().CountP("debug", "d", "show debug logs (or use LETS_DEBUG=1). If used multiple times, shows more verbose logs") //nolint:lll rootCmd.Flags().StringP("config", "c", "", "config file (default is lets.yaml)") rootCmd.Flags().Bool("all", false, "show all commands (including the ones with _)") + rootCmd.Flags().Bool("no-cache", false, "re-download remote config instead of using cached version") } diff --git a/internal/cmd/testdata/TestHelpGolden/basic.golden b/internal/cmd/testdata/TestHelpGolden/basic.golden index c818c5e7..29ebbdba 100644 --- a/internal/cmd/testdata/TestHelpGolden/basic.golden +++ b/internal/cmd/testdata/TestHelpGolden/basic.golden @@ -21,6 +21,7 @@ FLAGS --exclude Run all but excluded command(s) described in cmd as map -h --help Help for lets --init Create a new lets.yaml in the current folder + --no-cache Re-Download remote config instead of using cached version --no-depends Skip 'depends' for running command --only Run only specified command(s) described in cmd as map -v --version Version for lets diff --git a/internal/cmd/testdata/TestHelpGolden/grouped_commands.golden b/internal/cmd/testdata/TestHelpGolden/grouped_commands.golden index 4c58fd6e..f98be8de 100644 --- a/internal/cmd/testdata/TestHelpGolden/grouped_commands.golden +++ b/internal/cmd/testdata/TestHelpGolden/grouped_commands.golden @@ -26,6 +26,7 @@ FLAGS --exclude Run all but excluded command(s) described in cmd as map -h --help Help for lets --init Create a new lets.yaml in the current folder + --no-cache Re-Download remote config instead of using cached version --no-depends Skip 'depends' for running command --only Run only specified command(s) described in cmd as map -v --version Version for lets diff --git a/internal/cmd/testdata/TestHelpGolden/grouped_commands_long.golden b/internal/cmd/testdata/TestHelpGolden/grouped_commands_long.golden index ff0e0174..d546f8ea 100644 --- a/internal/cmd/testdata/TestHelpGolden/grouped_commands_long.golden +++ b/internal/cmd/testdata/TestHelpGolden/grouped_commands_long.golden @@ -27,6 +27,7 @@ FLAGS --exclude Run all but excluded command(s) described in cmd as map -h --help Help for lets --init Create a new lets.yaml in the current folder + --no-cache Re-Download remote config instead of using cached version --no-depends Skip 'depends' for running command --only Run only specified command(s) described in cmd as map -v --version Version for lets diff --git a/internal/cmd/testdata/TestHelpGolden/long_command.golden b/internal/cmd/testdata/TestHelpGolden/long_command.golden index f21c0521..78e3b850 100644 --- a/internal/cmd/testdata/TestHelpGolden/long_command.golden +++ b/internal/cmd/testdata/TestHelpGolden/long_command.golden @@ -22,6 +22,7 @@ FLAGS --exclude Run all but excluded command(s) described in cmd as map -h --help Help for lets --init Create a new lets.yaml in the current folder + --no-cache Re-Download remote config instead of using cached version --no-depends Skip 'depends' for running command --only Run only specified command(s) described in cmd as map -v --version Version for lets diff --git a/internal/config/config/config.go b/internal/config/config/config.go index 7070c2ce..ad014d3f 100644 --- a/internal/config/config/config.go +++ b/internal/config/config/config.go @@ -48,6 +48,9 @@ type Config struct { // absolute path to .lets/mixins MixinsDir string + // RemoteSource is the original URL when the config was loaded remotely; empty for local configs. + RemoteSource string + // cached env after config.SetupEnv, used in config.GetEnv cachedEnv map[string]string isMixin bool // if true, we consider config as mixin and apply different parsing and validation diff --git a/internal/config/config/mixin.go b/internal/config/config/mixin.go index 4b8b9c5f..2ce5a5ad 100644 --- a/internal/config/config/mixin.go +++ b/internal/config/config/mixin.go @@ -5,38 +5,14 @@ import ( "crypto/sha256" "encoding/hex" "fmt" - "io" - "mime" - "net/http" "os" "path/filepath" "strings" - "time" - "github.com/lets-cli/lets/internal/set" + "github.com/lets-cli/lets/internal/fetch" "github.com/lets-cli/lets/internal/util" ) -var allowedContentTypes = set.NewSet( - "text/plain", - "text/yaml", - "text/x-yaml", - "application/yaml", - "application/x-yaml", -) - -// normalizeContentType extracts the media type from a Content-Type header, -// removing parameters like charset to enable robust matching. -func normalizeContentType(contentType string) string { - mediaType, _, err := mime.ParseMediaType(contentType) - if err != nil { - // If parsing fails, return the original string - return contentType - } - - return mediaType -} - type Mixins []*Mixin type Mixin struct { @@ -102,50 +78,7 @@ func (rm *RemoteMixin) tryRead() ([]byte, error) { } func (rm *RemoteMixin) download() ([]byte, error) { - // TODO: maybe create a client for this? - ctx, cancel := context.WithTimeout(context.Background(), 60*5*time.Second) - defer cancel() - - req, err := http.NewRequestWithContext( - ctx, - http.MethodGet, - rm.URL, - nil, - ) - if err != nil { - return nil, err - } - - client := &http.Client{ - Timeout: 15 * 60 * time.Second, // TODO: move to client struct - } - - resp, err := client.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to make request: %w", err) - } - - defer resp.Body.Close() - - if resp.StatusCode == http.StatusNotFound { - return nil, fmt.Errorf("no such file at: %s", rm.URL) - } else if resp.StatusCode < 200 || resp.StatusCode > 299 { - return nil, fmt.Errorf("network error: %s", resp.Status) - } - - contentType := resp.Header.Get("Content-Type") - - normalizedContentType := normalizeContentType(contentType) - if !allowedContentTypes.Contains(normalizedContentType) { - return nil, fmt.Errorf("unsupported content type: %s", contentType) - } - - data, err := io.ReadAll(resp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read response: %w", err) - } - - return data, nil + return fetch.Download(context.Background(), rm.URL) } // Trim `-` prefix. diff --git a/internal/config/config/mixin_test.go b/internal/config/config/mixin_test.go index 77f42d17..d912156b 100644 --- a/internal/config/config/mixin_test.go +++ b/internal/config/config/mixin_test.go @@ -1,103 +1 @@ package config - -import ( - "testing" -) - -func TestNormalizeContentType(t *testing.T) { - tests := []struct { - name string - contentType string - expectedResult string - }{ - { - name: "simple content type", - contentType: "text/plain", - expectedResult: "text/plain", - }, - { - name: "content type with charset parameter", - contentType: "text/yaml; charset=utf-8", - expectedResult: "text/yaml", - }, - { - name: "content type with multiple parameters", - contentType: "application/yaml; charset=utf-8; boundary=something", - expectedResult: "application/yaml", - }, - { - name: "content type with quoted parameters", - contentType: `text/x-yaml; charset="utf-8"`, - expectedResult: "text/x-yaml", - }, - { - name: "invalid content type", - contentType: "invalid/content/type; malformed", - expectedResult: "invalid/content/type; malformed", - }, - { - name: "empty content type", - contentType: "", - expectedResult: "", - }, - { - name: "content type with spaces", - contentType: "text/plain ; charset=utf-8", - expectedResult: "text/plain", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := normalizeContentType(tt.contentType) - if result != tt.expectedResult { - t.Errorf("normalizeContentType(%q) = %q, want %q", tt.contentType, result, tt.expectedResult) - } - }) - } -} - -func TestAllowedContentTypes(t *testing.T) { - // Test that our normalization works with the allowed content types - testCases := []string{ - "text/plain", - "text/yaml", - "text/x-yaml", - "application/yaml", - "application/x-yaml", - } - - for _, contentType := range testCases { - // Test without parameters - if !allowedContentTypes.Contains(contentType) { - t.Errorf("allowedContentTypes should contain %q", contentType) - } - - // Test with charset parameter - withCharset := contentType + "; charset=utf-8" - normalized := normalizeContentType(withCharset) - if !allowedContentTypes.Contains(normalized) { - t.Errorf("normalized content type %q should be allowed (original: %q)", normalized, withCharset) - } - } - - // Test that disallowed content types are rejected - disallowedTypes := []string{ - "application/json", - "text/html", - "application/xml", - } - - for _, contentType := range disallowedTypes { - if allowedContentTypes.Contains(contentType) { - t.Errorf("allowedContentTypes should not contain %q", contentType) - } - - // Test with parameters - withCharset := contentType + "; charset=utf-8" - normalized := normalizeContentType(withCharset) - if allowedContentTypes.Contains(normalized) { - t.Errorf("normalized content type %q should not be allowed (original: %q)", normalized, withCharset) - } - } -} diff --git a/internal/config/config/runtime_env.go b/internal/config/config/runtime_env.go index 3159f3ea..bad9c5a8 100644 --- a/internal/config/config/runtime_env.go +++ b/internal/config/config/runtime_env.go @@ -7,9 +7,17 @@ import ( ) func (c *Config) BuiltinEnv(shell string) map[string]string { + letsConfig := filepath.Base(c.FilePath) + letsConfigDir := filepath.Dir(c.FilePath) + + if c.RemoteSource != "" { + letsConfig = c.RemoteSource + letsConfigDir = c.WorkDir + } + return map[string]string{ - "LETS_CONFIG": filepath.Base(c.FilePath), - "LETS_CONFIG_DIR": filepath.Dir(c.FilePath), + "LETS_CONFIG": letsConfig, + "LETS_CONFIG_DIR": letsConfigDir, "LETS_OS": runtime.GOOS, "LETS_ARCH": runtime.GOARCH, "LETS_SHELL": shell, diff --git a/internal/config/load.go b/internal/config/load.go index 6bf0d816..771d5cc7 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -1,10 +1,18 @@ package config import ( + "context" + "crypto/sha256" + "encoding/hex" "fmt" "os" + "path/filepath" "github.com/lets-cli/lets/internal/config/config" + "github.com/lets-cli/lets/internal/fetch" + "github.com/lets-cli/lets/internal/util" + "github.com/lets-cli/lets/internal/workdir" + log "github.com/sirupsen/logrus" "gopkg.in/yaml.v3" ) @@ -14,18 +22,53 @@ func Load(configName string, configDir string, version string) (*config.Config, return nil, err } - f, err := os.Open(configPath.AbsPath) + return loadConfigFromFile(configPath.AbsPath, configPath.WorkDir, configPath.DotLetsDir, configPath.Filename, version) +} + +// LoadRemote downloads (or loads from cache) a remote lets.yaml at url and +// returns a Config with the working directory set to the caller's CWD. +func LoadRemote(ctx context.Context, url string, noCache bool, version string) (*config.Config, error) { + cachedPath, err := ensureRemoteConfig(ctx, url, noCache) if err != nil { return nil, err } - c := config.NewConfig( - configPath.WorkDir, - configPath.AbsPath, - configPath.DotLetsDir, - ) + cwd, err := os.Getwd() + if err != nil { + return nil, fmt.Errorf("failed to get working directory: %w", err) + } + + dotLetsDir, err := workdir.GetDotLetsDir(cwd) + if err != nil { + return nil, fmt.Errorf("can not get .lets path: %w", err) + } + + if err := util.SafeCreateDir(dotLetsDir); err != nil { + return nil, fmt.Errorf("can not create .lets dir: %w", err) + } + + c, err := loadConfigFromFile(cachedPath, cwd, dotLetsDir, url, version) + if err != nil { + return nil, fmt.Errorf("%w (use --no-cache to re-download)", err) + } + + c.RemoteSource = url + + return c, nil +} + +// loadConfigFromFile is shared by Load and LoadRemote: opens the file at absPath, +// decodes YAML, validates, and sets up env. displayName appears in parse error messages. +func loadConfigFromFile(absPath, workDir, dotLetsDir, displayName, version string) (*config.Config, error) { + f, err := os.Open(absPath) + if err != nil { + return nil, err + } + defer f.Close() + + c := config.NewConfig(workDir, absPath, dotLetsDir) if err := yaml.NewDecoder(f).Decode(c); err != nil { - return nil, fmt.Errorf("failed to parse %s: %w", configPath.Filename, err) + return nil, fmt.Errorf("failed to parse %s: %w", displayName, err) } if err = validate(c, version); err != nil { @@ -38,3 +81,88 @@ func Load(configName string, configDir string, version string) (*config.Config, return c, nil } + +func ensureRemoteConfig(ctx context.Context, url string, noCache bool) (string, error) { + cacheDir, err := remoteConfigCacheDir() + if err != nil { + return "", err + } + + if err := os.MkdirAll(cacheDir, 0o755); err != nil { + return "", fmt.Errorf("can not create remote config cache dir: %w", err) + } + + cachePath := remoteConfigCachePath(cacheDir, url) + + if !noCache && util.FileExists(cachePath) { + return cachePath, nil + } + + data, downloadErr := fetch.Download(ctx, url) + if downloadErr != nil { + if util.FileExists(cachePath) { + log.Warnf("failed to download remote config (%v), falling back to cached version", downloadErr) + return cachePath, nil + } + + return "", fmt.Errorf("failed to download remote config: %w", downloadErr) + } + + if err := writeCacheAtomic(cachePath, data); err != nil { + return "", err + } + + return cachePath, nil +} + +// writeCacheAtomic writes data to a sibling temp file then renames it to dst, +// ensuring the cache path is never left in a partially-written state. +func writeCacheAtomic(dst string, data []byte) error { + dir := filepath.Dir(dst) + + tmp, err := os.CreateTemp(dir, "*.yaml.tmp") + if err != nil { + return fmt.Errorf("failed to create temp cache file: %w", err) + } + + tmpPath := tmp.Name() + + _, writeErr := tmp.Write(data) + closeErr := tmp.Close() + + if writeErr != nil || closeErr != nil { + os.Remove(tmpPath) + if writeErr != nil { + return fmt.Errorf("failed to write temp cache file: %w", writeErr) + } + + return fmt.Errorf("failed to close temp cache file: %w", closeErr) + } + + //#nosec G306 + if err := os.Chmod(tmpPath, 0o644); err != nil { + os.Remove(tmpPath) + return fmt.Errorf("failed to chmod temp cache file: %w", err) + } + + if err := os.Rename(tmpPath, dst); err != nil { + os.Remove(tmpPath) + return fmt.Errorf("failed to cache remote config: %w", err) + } + + return nil +} + +func remoteConfigCacheDir() (string, error) { + userDir, err := util.LetsUserDir() + if err != nil { + return "", err + } + + return filepath.Join(userDir, "remote-configs"), nil +} + +func remoteConfigCachePath(cacheDir, url string) string { + hash := sha256.Sum256([]byte(url)) + return filepath.Join(cacheDir, hex.EncodeToString(hash[:])+".yaml") +} diff --git a/internal/config/load_test.go b/internal/config/load_test.go index 3b92064e..da2df7ed 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -1,6 +1,9 @@ package config import ( + "context" + "net/http" + "net/http/httptest" "os" "path/filepath" "strings" @@ -42,3 +45,121 @@ func TestLoadConfig(t *testing.T) { } }) } + +func TestLoadRemote(t *testing.T) { + validConfig := "shell: bash\ncommands:\n hello:\n cmd: echo hello\n" + ctx := context.Background() + + t.Run("downloads and caches config", func(t *testing.T) { + requests := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + w.Header().Set("Content-Type", "application/yaml") + _, _ = w.Write([]byte(validConfig)) + })) + defer srv.Close() + + t.Setenv("HOME", t.TempDir()) + t.Chdir(t.TempDir()) + + cfg, err := LoadRemote(ctx, srv.URL, false, "0.0.0-test") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, ok := cfg.Commands["hello"]; !ok { + t.Fatal("expected hello command") + } + if cfg.RemoteSource != srv.URL { + t.Fatalf("expected RemoteSource=%q, got %q", srv.URL, cfg.RemoteSource) + } + if requests != 1 { + t.Fatalf("expected 1 HTTP request, got %d", requests) + } + }) + + t.Run("uses cache on second call without --no-cache", func(t *testing.T) { + requests := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + w.Header().Set("Content-Type", "application/yaml") + _, _ = w.Write([]byte(validConfig)) + })) + defer srv.Close() + + t.Setenv("HOME", t.TempDir()) + t.Chdir(t.TempDir()) + + if _, err := LoadRemote(ctx, srv.URL, false, "0.0.0-test"); err != nil { + t.Fatalf("first call error: %v", err) + } + if _, err := LoadRemote(ctx, srv.URL, false, "0.0.0-test"); err != nil { + t.Fatalf("second call error: %v", err) + } + if requests != 1 { + t.Fatalf("expected 1 HTTP request, got %d", requests) + } + }) + + t.Run("re-downloads with --no-cache", func(t *testing.T) { + requests := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + w.Header().Set("Content-Type", "application/yaml") + _, _ = w.Write([]byte(validConfig)) + })) + defer srv.Close() + + t.Setenv("HOME", t.TempDir()) + t.Chdir(t.TempDir()) + + if _, err := LoadRemote(ctx, srv.URL, false, "0.0.0-test"); err != nil { + t.Fatalf("prime cache error: %v", err) + } + if _, err := LoadRemote(ctx, srv.URL, true, "0.0.0-test"); err != nil { + t.Fatalf("no-cache call error: %v", err) + } + if requests != 2 { + t.Fatalf("expected 2 HTTP requests, got %d", requests) + } + }) + + t.Run("falls back to cache when --no-cache download fails", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/yaml") + _, _ = w.Write([]byte(validConfig)) + })) + + t.Setenv("HOME", t.TempDir()) + t.Chdir(t.TempDir()) + url := srv.URL + + if _, err := LoadRemote(ctx, url, false, "0.0.0-test"); err != nil { + t.Fatalf("prime cache error: %v", err) + } + + srv.Close() + + cfg, err := LoadRemote(ctx, url, true, "0.0.0-test") + if err != nil { + t.Fatalf("expected fallback to cache, got error: %v", err) + } + if _, ok := cfg.Commands["hello"]; !ok { + t.Fatal("expected hello command from cache") + } + }) + + t.Run("errors when download fails with no cache", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + srv.Close() + + t.Setenv("HOME", t.TempDir()) + t.Chdir(t.TempDir()) + + _, err := LoadRemote(ctx, srv.URL, false, "0.0.0-test") + if err == nil { + t.Fatal("expected error when no cache and download fails") + } + }) +} diff --git a/internal/fetch/fetch.go b/internal/fetch/fetch.go new file mode 100644 index 00000000..dde85fb8 --- /dev/null +++ b/internal/fetch/fetch.go @@ -0,0 +1,65 @@ +package fetch + +import ( + "context" + "fmt" + "io" + "mime" + "net/http" + "time" + + "github.com/lets-cli/lets/internal/set" +) + +var allowedContentTypes = set.NewSet( + "text/plain", + "text/yaml", + "text/x-yaml", + "application/yaml", + "application/x-yaml", +) + +// httpClient backstop timeout guards against hung connections when callers pass context.Background(). +// 5 minutes matches the previous per-request context timeout used by RemoteMixin downloads. +var httpClient = &http.Client{ + Timeout: 5 * 60 * time.Second, +} + +// Download fetches the content at url, validates the Content-Type is a YAML variant, +// and returns the raw bytes. +func Download(ctx context.Context, url string) ([]byte, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, err + } + + resp, err := httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to make request: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode == http.StatusNotFound { + return nil, fmt.Errorf("no such file at: %s", url) + } + if resp.StatusCode < 200 || resp.StatusCode > 299 { + return nil, fmt.Errorf("network error for %s: %s", url, resp.Status) + } + + contentType := resp.Header.Get("Content-Type") + mediaType, _, parseErr := mime.ParseMediaType(contentType) + if parseErr != nil { + mediaType = contentType + } + + if !allowedContentTypes.Contains(mediaType) { + return nil, fmt.Errorf("unsupported content type: %s", contentType) + } + + data, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("failed to read response: %w", err) + } + + return data, nil +} diff --git a/internal/fetch/fetch_test.go b/internal/fetch/fetch_test.go new file mode 100644 index 00000000..ac104a53 --- /dev/null +++ b/internal/fetch/fetch_test.go @@ -0,0 +1,87 @@ +package fetch_test + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/lets-cli/lets/internal/fetch" +) + +func TestDownload(t *testing.T) { + t.Run("downloads valid yaml", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/yaml") + _, _ = w.Write([]byte("shell: bash\ncommands: {}")) + })) + defer srv.Close() + + data, err := fetch.Download(t.Context(), srv.URL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if string(data) != "shell: bash\ncommands: {}" { + t.Fatalf("unexpected data: %s", data) + } + }) + + t.Run("accepts text/plain content type", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + _, _ = w.Write([]byte("shell: bash")) + })) + defer srv.Close() + + _, err := fetch.Download(t.Context(), srv.URL) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("errors on unsupported content type", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte("{}")) + })) + defer srv.Close() + + _, err := fetch.Download(t.Context(), srv.URL) + if err == nil { + t.Fatal("expected error for unsupported content type") + } + if !strings.Contains(err.Error(), "unsupported content type") { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("errors on 404", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + })) + defer srv.Close() + + _, err := fetch.Download(t.Context(), srv.URL) + if err == nil { + t.Fatal("expected error for 404") + } + if !strings.Contains(err.Error(), "no such file at") { + t.Fatalf("unexpected error: %v", err) + } + }) + + t.Run("errors on non-2xx", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + _, err := fetch.Download(t.Context(), srv.URL) + if err == nil { + t.Fatal("expected error for 500") + } + if !strings.Contains(err.Error(), "network error") { + t.Fatalf("unexpected error: %v", err) + } + }) +}