diff --git a/internal/config/appconfig.go b/internal/config/appconfig.go index 3dc4d30..478ef3d 100644 --- a/internal/config/appconfig.go +++ b/internal/config/appconfig.go @@ -15,6 +15,9 @@ type AppConfig struct { RunLogRetentionDays int `json:"run_log_retention_days"` RunRetentionEnabled bool `json:"run_retention_enabled"` RunRetentionDays int `json:"run_retention_days"` + // DefaultTags are org-wide default tags merged beneath each pack's own + // default_tags parameter when pack configs are built from the DB. + DefaultTags map[string]string `json:"default_tags"` } // DefaultAppConfig returns the default values used when no row exists for @@ -29,5 +32,6 @@ func DefaultAppConfig() AppConfig { RunLogRetentionDays: 7, RunRetentionEnabled: false, RunRetentionDays: 30, + DefaultTags: map[string]string{}, } } diff --git a/internal/config/appconfig_test.go b/internal/config/appconfig_test.go index fadc06d..6d5438a 100644 --- a/internal/config/appconfig_test.go +++ b/internal/config/appconfig_test.go @@ -16,5 +16,6 @@ func TestDefaultAppConfig(t *testing.T) { RunLogRetentionDays: 7, RunRetentionEnabled: false, RunRetentionDays: 30, + DefaultTags: map[string]string{}, }, DefaultAppConfig()) } diff --git a/internal/db/config.go b/internal/db/config.go index c29599c..b452b49 100644 --- a/internal/db/config.go +++ b/internal/db/config.go @@ -120,6 +120,12 @@ func parseAppConfig(all map[string]json.RawMessage) config.AppConfig { cfg.RunRetentionDays = n } } + if v, ok := all["default_tags"]; ok { + var m map[string]string + if err := json.Unmarshal(v, &m); err == nil && m != nil { + cfg.DefaultTags = m + } + } return cfg } @@ -142,6 +148,7 @@ func appConfigKVs(c config.AppConfig) []appConfigKV { {"run_log_retention_days", c.RunLogRetentionDays}, {"run_retention_enabled", c.RunRetentionEnabled}, {"run_retention_days", c.RunRetentionDays}, + {"default_tags", c.DefaultTags}, } } diff --git a/internal/db/config_test.go b/internal/db/config_test.go index a63a490..42055c4 100644 --- a/internal/db/config_test.go +++ b/internal/db/config_test.go @@ -30,6 +30,7 @@ func TestParseAppConfig_InvalidJSONKeepsDefault(t *testing.T) { "terraform_version": json.RawMessage(`123`), "pack_logs_enabled": json.RawMessage(`"yes"`), "ssh_logging_enabled": json.RawMessage(`{bad json`), + "default_tags": json.RawMessage(`"owner=secops"`), }) assert.Equal(t, config.DefaultAppConfig(), got) } @@ -55,6 +56,7 @@ func TestParseAppConfig_AllSet(t *testing.T) { "run_log_retention_days": json.RawMessage(`14`), "run_retention_enabled": json.RawMessage(`true`), "run_retention_days": json.RawMessage(`90`), + "default_tags": json.RawMessage(`{"owner":"secops"}`), }) assert.Equal(t, config.AppConfig{ Parallelism: 12, @@ -65,6 +67,7 @@ func TestParseAppConfig_AllSet(t *testing.T) { RunLogRetentionDays: 14, RunRetentionEnabled: true, RunRetentionDays: 90, + DefaultTags: map[string]string{"owner": "secops"}, }, got) } @@ -94,6 +97,7 @@ func TestAppConfigKVs_MarshalsToExpectedJSON(t *testing.T) { RunLogRetentionDays: 7, RunRetentionEnabled: false, RunRetentionDays: 30, + DefaultTags: map[string]string{"owner": "secops"}, } want := map[string]string{ @@ -105,6 +109,7 @@ func TestAppConfigKVs_MarshalsToExpectedJSON(t *testing.T) { "run_log_retention_days": `7`, "run_retention_enabled": `false`, "run_retention_days": `30`, + "default_tags": `{"owner":"secops"}`, } kvs := appConfigKVs(c) @@ -129,6 +134,7 @@ func TestFakeConfigStore_UpdateGetAppConfigRoundtrip(t *testing.T) { RunLogRetentionDays: 14, RunRetentionEnabled: true, RunRetentionDays: 90, + DefaultTags: map[string]string{"owner": "secops", "simulated": "true"}, } require.NoError(t, f.UpdateAppConfig(ctx, want)) @@ -139,7 +145,7 @@ func TestFakeConfigStore_UpdateGetAppConfigRoundtrip(t *testing.T) { assert.Equal(t, []string{ "parallelism", "terraform_version", "pack_logs_enabled", "ssh_logging_enabled", "run_log_retention_enabled", "run_log_retention_days", - "run_retention_enabled", "run_retention_days", + "run_retention_enabled", "run_retention_days", "default_tags", }, f.sets) } diff --git a/internal/db/migrations/018_default_tags_appconfig.down.sql b/internal/db/migrations/018_default_tags_appconfig.down.sql new file mode 100644 index 0000000..ac288c5 --- /dev/null +++ b/internal/db/migrations/018_default_tags_appconfig.down.sql @@ -0,0 +1 @@ +DELETE FROM app_config WHERE key = 'default_tags'; diff --git a/internal/db/migrations/018_default_tags_appconfig.up.sql b/internal/db/migrations/018_default_tags_appconfig.up.sql new file mode 100644 index 0000000..5ba2442 --- /dev/null +++ b/internal/db/migrations/018_default_tags_appconfig.up.sql @@ -0,0 +1,5 @@ +-- Backfill app_config with the org-wide default_tags key. +-- Idempotent — only inserts if the key is missing. Aligned with DefaultAppConfig(). +INSERT INTO app_config (key, value) VALUES + ('default_tags', '{}'::jsonb) +ON CONFLICT (key) DO NOTHING; diff --git a/internal/detonators/simrun_detonator_test.go b/internal/detonators/simrun_detonator_test.go index a244419..29f8d6c 100644 --- a/internal/detonators/simrun_detonator_test.go +++ b/internal/detonators/simrun_detonator_test.go @@ -108,6 +108,23 @@ func TestTerraformEnvVars_MapValueJSONEncoded(t *testing.T) { } } +// The org-wide default tags merge (web.loadPacksFromDB) stores default_tags +// as map[string]string; it must reach terraform identically to the +// map[string]any a pack stores on its own. +func TestTerraformEnvVars_MergedStringMapJSONEncoded(t *testing.T) { + d := &SimrunDetonator{ + packConfig: config.PackConfig{ + Parameters: map[string]any{ + "default_tags": map[string]string{"owner": "secops"}, + }, + }, + } + env := d.terraformEnvVars("exec-1") + if got := env["TF_VAR_default_tags"]; got != `{"owner":"secops"}` { + t.Errorf("TF_VAR_default_tags = %q, want JSON-encoded object", got) + } +} + func TestFormatTFVar(t *testing.T) { tests := []struct { in any diff --git a/internal/packs/resolver/resolver.go b/internal/packs/resolver/resolver.go index 6d6c660..c9afb4f 100644 --- a/internal/packs/resolver/resolver.go +++ b/internal/packs/resolver/resolver.go @@ -23,6 +23,11 @@ import ( "github.com/sirupsen/logrus" ) +// defaultGitHubAPIBaseURL is the public GitHub Releases API base. It can be +// overridden via SR_GITHUB_API_URL (used by tests and, incidentally, for +// GitHub Enterprise). +const defaultGitHubAPIBaseURL = "https://api.github.com" + // PackConfig represents configuration for a remote pack to resolve. type PackConfig struct { Name string @@ -30,23 +35,25 @@ type PackConfig struct { Version string } +// FetchResult is returned by Fetch after a remote pack has been downloaded, +// verified, and extracted. +type FetchResult struct { + // Version is the concrete resolved release tag with any leading "v" stripped. + Version string + // BinaryPath is the absolute path to the extracted pack binary. + BinaryPath string +} + // Resolver downloads, caches, and resolves pack binaries. type Resolver struct { cacheDir string httpClient *http.Client + apiBaseURL string } // NewResolver creates a new Resolver caching packs under /packs/. func NewResolver(dataDir string) (*Resolver, error) { - cacheDir := filepath.Join(dataDir, "packs") - if err := os.MkdirAll(cacheDir, 0755); err != nil { - return nil, fmt.Errorf("failed to create cache directory: %w", err) - } - - return &Resolver{ - cacheDir: cacheDir, - httpClient: &http.Client{}, - }, nil + return NewResolverWithCacheDir(filepath.Join(dataDir, "packs")) } // NewResolverWithCacheDir creates a new Resolver with a custom cache directory. @@ -55,9 +62,15 @@ func NewResolverWithCacheDir(cacheDir string) (*Resolver, error) { return nil, fmt.Errorf("failed to create cache directory: %w", err) } + apiBaseURL := defaultGitHubAPIBaseURL + if v := os.Getenv("SR_GITHUB_API_URL"); v != "" { + apiBaseURL = strings.TrimRight(v, "/") + } + return &Resolver{ cacheDir: cacheDir, httpClient: &http.Client{}, + apiBaseURL: apiBaseURL, }, nil } @@ -67,13 +80,14 @@ func (r *Resolver) CacheDir() string { } // Resolve returns the path to a remote pack binary, downloading if needed. +// At runtime the binary is normally already cached (populated at install), so +// this is a cache hit; on a miss it re-fetches the pinned release. func (r *Resolver) Resolve(ctx context.Context, cfg PackConfig) (string, error) { if err := r.validatePackConfig(cfg); err != nil { return "", err } - cachedPath := r.getCachedPath(cfg) - if r.isCached(cachedPath) { + if cachedPath, ok := r.cachedBinary(cfg); ok { logrus.WithField("pack", cfg.Name).WithField("version", cfg.Version).WithField("path", cachedPath).Debug("Using cached pack binary") return cachedPath, nil } @@ -84,19 +98,29 @@ func (r *Resolver) Resolve(ctx context.Context, cfg PackConfig) (string, error) defer release() // Double-checked: another waiter may have downloaded it while we blocked. - if r.isCached(cachedPath) { + if cachedPath, ok := r.cachedBinary(cfg); ok { logrus.WithField("pack", cfg.Name).WithField("version", cfg.Version).WithField("path", cachedPath).Debug("Using cached pack binary") return cachedPath, nil } - if err := r.download(ctx, cfg); err != nil { + destDir := filepath.Join(r.cacheDir, cfg.Name, cfg.Version) + if err := os.MkdirAll(destDir, 0755); err != nil { + return "", fmt.Errorf("failed to create cache directory: %w", err) + } + + res, err := r.Fetch(ctx, cfg.Source, cfg.Version, destDir) + if err != nil { return "", fmt.Errorf("failed to download pack %s: %w", cfg.Name, err) } - return cachedPath, nil + logrus.WithField("pack", cfg.Name).WithField("version", res.Version).WithField("path", res.BinaryPath).Info("Pack downloaded and cached") + return res.BinaryPath, nil } -// validatePackConfig validates the pack configuration. +// validatePackConfig validates the pack configuration. Version must be a +// concrete pinned version: cache paths are ///, so an +// empty version would read/write the pack's root cache dir. Install pins the +// resolved tag, so this only rejects malformed (e.g. legacy) rows. func (r *Resolver) validatePackConfig(cfg PackConfig) error { if cfg.Source == "" { return fmt.Errorf("pack %s: source is required", cfg.Name) @@ -107,13 +131,25 @@ func (r *Resolver) validatePackConfig(cfg PackConfig) error { return nil } -// isCached checks if a file exists at the given path. -func (r *Resolver) isCached(path string) bool { - _, err := os.Stat(path) - return err == nil +// cachedBinary returns the path to the cached binary for cfg, if present. The +// binary's on-disk name is the manifest-derived name picked at install time, so +// it is located by scanning the version directory rather than assuming a name. +func (r *Resolver) cachedBinary(cfg PackConfig) (string, bool) { + dir := filepath.Join(r.cacheDir, cfg.Name, cfg.Version) + entries, err := os.ReadDir(dir) + if err != nil { + return "", false + } + for _, e := range entries { + if !e.IsDir() { + return filepath.Join(dir, e.Name()), true + } + } + return "", false } -// GetManifest calls the pack's manifest command and parses the response. +// GetManifest calls the pack's manifest command and parses the response. This +// is the shared helper used at install time to derive a pack's identity. func (r *Resolver) GetManifest(ctx context.Context, packPath string) (*pack.ManifestResponse, error) { cmd := exec.CommandContext(ctx, packPath, "manifest") @@ -133,66 +169,194 @@ func (r *Resolver) GetManifest(ctx context.Context, packPath string) (*pack.Mani return &manifest, nil } -// getCachedPath returns the path where a pack binary should be cached. -func (r *Resolver) getCachedPath(cfg PackConfig) string { - return filepath.Join(r.cacheDir, cfg.Name, cfg.Version, cfg.Name) -} +// Fetch resolves a remote pack release via the GitHub Releases API, downloads +// and checksum-verifies the platform archive, and extracts the binary into +// destDir. version may be empty to resolve the latest release. It returns the +// concrete resolved version and the extracted binary path. +func (r *Resolver) Fetch(ctx context.Context, source, version, destDir string) (FetchResult, error) { + org, repo, err := parseGitHubSource(source) + if err != nil { + return FetchResult{}, err + } -// download downloads a pack from GitHub releases. -func (r *Resolver) download(ctx context.Context, cfg PackConfig) error { - org, repo, err := parseGitHubSource(cfg.Source) + rel, err := r.resolveRelease(ctx, org, repo, version) if err != nil { - return err + return FetchResult{}, err } - urls := r.buildDownloadURLs(org, repo, cfg) - logrus.WithField("pack", cfg.Name).WithField("version", cfg.Version).WithField("url", urls.archive).Info("Downloading pack") + archive, checksums, err := selectAssets(rel.Assets, rel.TagName) + if err != nil { + return FetchResult{}, err + } + + logrus.WithField("source", source).WithField("tag", rel.TagName).WithField("asset", archive.Name).Info("Downloading pack") - archiveData, err := r.downloadAndVerify(ctx, urls) + archiveData, err := r.downloadAndVerify(ctx, archive, checksums) if err != nil { - return err + return FetchResult{}, err } - if err := r.extractAndCache(archiveData, cfg); err != nil { - return err + binaryName, err := extractBinary(archiveData, destDir, binaryNameFromAsset(archive.Name)) + if err != nil { + return FetchResult{}, fmt.Errorf("failed to extract archive: %w", err) } - logrus.WithField("pack", cfg.Name).WithField("version", cfg.Version).WithField("path", r.getCachedPath(cfg)).Info("Pack downloaded and cached") - return nil + return FetchResult{ + Version: strings.TrimPrefix(rel.TagName, "v"), + BinaryPath: filepath.Join(destDir, binaryName), + }, nil +} + +// githubRelease is the subset of the GitHub Releases API response we use. +type githubRelease struct { + TagName string `json:"tag_name"` + Assets []githubAsset `json:"assets"` } -// downloadURLs contains URLs for downloading a pack. -type downloadURLs struct { - archive string - checksums string - archiveName string +// githubAsset is a single release asset. +type githubAsset struct { + Name string `json:"name"` + BrowserDownloadURL string `json:"browser_download_url"` +} + +// resolveRelease fetches release metadata from the GitHub Releases API. An +// empty version resolves the latest release. Otherwise the version is treated +// as a bare version regardless of how the operator typed it ("v0.0.1" and +// "0.0.1" are equivalent): the conventional "v"-prefixed tag is tried first, +// then the bare tag, so both common tagging conventions resolve. +func (r *Resolver) resolveRelease(ctx context.Context, org, repo, version string) (*githubRelease, error) { + if version == "" { + rel, notFound, err := r.getRelease(ctx, fmt.Sprintf("%s/repos/%s/%s/releases/latest", r.apiBaseURL, org, repo), org, repo) + if err != nil { + return nil, err + } + if notFound { + return nil, fmt.Errorf("no published release found for %s/%s", org, repo) + } + return rel, nil + } + + bare := strings.TrimPrefix(version, "v") + for _, tag := range []string{"v" + bare, bare} { + url := fmt.Sprintf("%s/repos/%s/%s/releases/tags/%s", r.apiBaseURL, org, repo, tag) + rel, notFound, err := r.getRelease(ctx, url, org, repo) + if err != nil { + return nil, err + } + if !notFound { + return rel, nil + } + } + return nil, fmt.Errorf("release not found for %s/%s: no tag %q or %q exists", org, repo, "v"+bare, bare) +} + +// getRelease performs a single Releases API GET. It reports notFound==true for a +// 404 (so the caller can try an alternative tag spelling) and returns a clear +// error for rate-limit and other non-OK responses. +func (r *Resolver) getRelease(ctx context.Context, url, org, repo string) (rel *githubRelease, notFound bool, err error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, false, err + } + req.Header.Set("Accept", "application/vnd.github+json") + + resp, err := r.httpClient.Do(req) + if err != nil { + return nil, false, err + } + defer resp.Body.Close() + + switch resp.StatusCode { + case http.StatusOK: + // proceed + case http.StatusNotFound: + return nil, true, nil + case http.StatusForbidden, http.StatusTooManyRequests: + return nil, false, fmt.Errorf("GitHub API rate limit exceeded resolving %s/%s: retry later or pin an explicit version", org, repo) + default: + return nil, false, fmt.Errorf("GitHub API returned HTTP %d resolving %s/%s", resp.StatusCode, org, repo) + } + + var decoded githubRelease + if err := json.NewDecoder(resp.Body).Decode(&decoded); err != nil { + return nil, false, fmt.Errorf("failed to parse GitHub release response: %w", err) + } + if decoded.TagName == "" { + return nil, false, fmt.Errorf("GitHub release for %s/%s has no tag", org, repo) + } + return &decoded, false, nil } -// buildDownloadURLs builds the download URLs for a pack. -func (r *Resolver) buildDownloadURLs(org, repo string, cfg PackConfig) downloadURLs { - archiveName := fmt.Sprintf("%s_%s_%s_%s.tar.gz", cfg.Name, cfg.Version, runtime.GOOS, runtime.GOARCH) - baseURL := fmt.Sprintf("https://github.com/%s/%s/releases/download/v%s", org, repo, cfg.Version) +// selectAssets picks the platform archive (matching *__.tar.gz) and +// the checksums asset from a release's asset list. +func selectAssets(assets []githubAsset, tag string) (archive, checksums githubAsset, err error) { + suffix := fmt.Sprintf("_%s_%s.tar.gz", runtime.GOOS, runtime.GOARCH) + + var matches []githubAsset + for _, a := range assets { + if strings.HasSuffix(a.Name, suffix) { + matches = append(matches, a) + } + } + + switch len(matches) { + case 0: + return githubAsset{}, githubAsset{}, fmt.Errorf("no asset matching *%s for %s/%s in release %s", suffix, runtime.GOOS, runtime.GOARCH, tag) + case 1: + archive = matches[0] + default: + names := make([]string, len(matches)) + for i, a := range matches { + names[i] = a.Name + } + return githubAsset{}, githubAsset{}, fmt.Errorf("multiple assets match *%s in release %s: %s", suffix, tag, strings.Join(names, ", ")) + } + + checksums, ok := findChecksumsAsset(assets) + if !ok { + return githubAsset{}, githubAsset{}, fmt.Errorf("no checksums.txt asset found in release %s", tag) + } + return archive, checksums, nil +} - return downloadURLs{ - archive: fmt.Sprintf("%s/%s", baseURL, archiveName), - checksums: fmt.Sprintf("%s/checksums.txt", baseURL), - archiveName: archiveName, +// findChecksumsAsset locates the checksums asset, preferring an exact +// "checksums.txt" name and falling back to any asset ending in "checksums.txt". +func findChecksumsAsset(assets []githubAsset) (githubAsset, bool) { + for _, a := range assets { + if a.Name == "checksums.txt" { + return a, true + } + } + for _, a := range assets { + if strings.HasSuffix(a.Name, "checksums.txt") { + return a, true + } } + return githubAsset{}, false +} + +// binaryNameFromAsset derives the expected in-tarball binary name from an +// archive asset name of the form ___.tar.gz. This is a +// best-effort hint; extraction falls back to the single executable in the +// archive when no entry matches. +func binaryNameFromAsset(assetName string) string { + return strings.SplitN(assetName, "_", 2)[0] } -// downloadAndVerify downloads an archive and verifies its checksum. -func (r *Resolver) downloadAndVerify(ctx context.Context, urls downloadURLs) ([]byte, error) { - checksums, err := r.downloadChecksums(ctx, urls.checksums) +// downloadAndVerify downloads an archive asset and verifies its checksum against +// the release's checksums asset. +func (r *Resolver) downloadAndVerify(ctx context.Context, archive, checksumsAsset githubAsset) ([]byte, error) { + checksums, err := r.downloadChecksums(ctx, checksumsAsset.BrowserDownloadURL) if err != nil { return nil, fmt.Errorf("failed to download checksums: %w", err) } - expectedChecksum, ok := checksums[urls.archiveName] + expectedChecksum, ok := checksums[archive.Name] if !ok { - return nil, fmt.Errorf("checksum not found for %s", urls.archiveName) + return nil, fmt.Errorf("checksum not found for %s", archive.Name) } - archiveData, err := r.downloadFile(ctx, urls.archive) + archiveData, err := r.downloadFile(ctx, archive.BrowserDownloadURL) if err != nil { return nil, fmt.Errorf("failed to download archive: %w", err) } @@ -214,20 +378,6 @@ func (r *Resolver) verifyChecksum(data []byte, expected string) error { return nil } -// extractAndCache extracts an archive and caches it. -func (r *Resolver) extractAndCache(archiveData []byte, cfg PackConfig) error { - cacheDir := filepath.Join(r.cacheDir, cfg.Name, cfg.Version) - if err := os.MkdirAll(cacheDir, 0755); err != nil { - return fmt.Errorf("failed to create cache directory: %w", err) - } - - if err := r.extractTarGz(archiveData, cacheDir, cfg.Name); err != nil { - return fmt.Errorf("failed to extract archive: %w", err) - } - - return nil -} - // downloadFile downloads a file from a URL. func (r *Resolver) downloadFile(ctx context.Context, url string) ([]byte, error) { req, err := http.NewRequestWithContext(ctx, "GET", url, nil) @@ -292,49 +442,96 @@ func (r *Resolver) parseChecksumLine(line string) (checksum, filename string) { return strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]) } -// extractTarGz extracts a .tar.gz archive. -func (r *Resolver) extractTarGz(data []byte, destDir string, expectedBinaryName string) error { +// extractBinary extracts the pack binary from a .tar.gz archive into destDir. +// It prefers the entry whose base name == preferredName (derived from the asset +// prefix); otherwise it falls back to the single executable in the archive, or +// the sole regular file. Returns the extracted binary's base name. +func extractBinary(data []byte, destDir, preferredName string) (string, error) { + target, err := chooseArchiveBinary(data, preferredName) + if err != nil { + return "", err + } + gzReader, err := gzip.NewReader(bytes.NewReader(data)) if err != nil { - return fmt.Errorf("failed to create gzip reader: %w", err) + return "", fmt.Errorf("failed to create gzip reader: %w", err) } defer gzReader.Close() tarReader := tar.NewReader(gzReader) - for { header, err := tarReader.Next() if err == io.EOF { break } if err != nil { - return fmt.Errorf("failed to read tar entry: %w", err) + return "", fmt.Errorf("failed to read tar entry: %w", err) } - - // Only extract the binary file - baseName := filepath.Base(header.Name) - if baseName != expectedBinaryName { + if header.Name != target { continue } - destPath := filepath.Join(destDir, expectedBinaryName) + baseName := filepath.Base(target) + destPath := filepath.Join(destDir, baseName) outFile, err := os.OpenFile(destPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0755) if err != nil { - return fmt.Errorf("failed to create file: %w", err) + return "", fmt.Errorf("failed to create file: %w", err) } - - if _, err := io.Copy(outFile, tarReader); err != nil { + if _, err := io.Copy(outFile, tarReader); err != nil { //nolint:gosec // archive is checksum-verified before extraction outFile.Close() - return fmt.Errorf("failed to write file: %w", err) + return "", fmt.Errorf("failed to write file: %w", err) } if err := outFile.Close(); err != nil { - return fmt.Errorf("failed to finalize file: %w", err) + return "", fmt.Errorf("failed to finalize file: %w", err) } + return baseName, nil + } + + return "", fmt.Errorf("binary %q not found in archive", target) +} - return nil +// chooseArchiveBinary scans the archive's headers and decides which entry is the +// pack binary, without reading entry contents. +func chooseArchiveBinary(data []byte, preferredName string) (string, error) { + gzReader, err := gzip.NewReader(bytes.NewReader(data)) + if err != nil { + return "", fmt.Errorf("failed to create gzip reader: %w", err) } + defer gzReader.Close() - return fmt.Errorf("binary %s not found in archive", expectedBinaryName) + tarReader := tar.NewReader(gzReader) + var regularFiles []string + var execFiles []string + for { + header, err := tarReader.Next() + if err == io.EOF { + break + } + if err != nil { + return "", fmt.Errorf("failed to read tar entry: %w", err) + } + if header.Typeflag != tar.TypeReg { + continue + } + if preferredName != "" && filepath.Base(header.Name) == preferredName { + return header.Name, nil + } + regularFiles = append(regularFiles, header.Name) + if header.Mode&0111 != 0 { + execFiles = append(execFiles, header.Name) + } + } + + if len(execFiles) == 1 { + return execFiles[0], nil + } + if len(regularFiles) == 1 { + return regularFiles[0], nil + } + if len(regularFiles) == 0 { + return "", fmt.Errorf("archive contains no files") + } + return "", fmt.Errorf("cannot determine pack binary in archive: %d candidate files and no unique executable (looked for %q)", len(regularFiles), preferredName) } // parseGitHubSource parses a GitHub source string (e.g., "github.com/org/repo"). diff --git a/internal/packs/resolver/resolver_concurrency_test.go b/internal/packs/resolver/resolver_concurrency_test.go index d67cc23..14aecef 100644 --- a/internal/packs/resolver/resolver_concurrency_test.go +++ b/internal/packs/resolver/resolver_concurrency_test.go @@ -7,10 +7,10 @@ import ( "context" "crypto/sha256" "encoding/hex" + "encoding/json" "fmt" "net/http" "net/http/httptest" - "net/url" "os" "runtime" "strings" @@ -19,18 +19,6 @@ import ( "testing" ) -// redirectTransport rewrites the scheme+host of every request to the stub -// server, so resolver's hardcoded github.com URLs reach the test server. -type redirectTransport struct { - target *url.URL -} - -func (t *redirectTransport) RoundTrip(req *http.Request) (*http.Response, error) { - req.URL.Scheme = t.target.Scheme - req.URL.Host = t.target.Host - return http.DefaultTransport.RoundTrip(req) -} - // TestResolveConcurrentSamePackDownloadsOnce verifies that two concurrent // Resolve calls for the same pack serialize so the archive is downloaded // exactly once and the cached binary is complete. Without the per-pack lock @@ -49,31 +37,32 @@ func TestResolveConcurrentSamePackDownloadsOnce(t *testing.T) { checksums := fmt.Sprintf("%s %s\n", hex.EncodeToString(sum[:]), archiveName) var archiveDownloads int32 + var srv *httptest.Server mux := http.NewServeMux() mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { switch { + case strings.HasSuffix(r.URL.Path, "/releases/tags/v"+version): + writeReleaseJSON(w, srv.URL, "v"+version, []githubAsset{ + {Name: archiveName}, + {Name: "checksums.txt"}, + }) case strings.HasSuffix(r.URL.Path, "/checksums.txt"): - w.Write([]byte(checksums)) + _, _ = w.Write([]byte(checksums)) case strings.HasSuffix(r.URL.Path, archiveName): atomic.AddInt32(&archiveDownloads, 1) - w.Write(archive) + _, _ = w.Write(archive) default: http.NotFound(w, r) } }) - srv := httptest.NewServer(mux) + srv = httptest.NewServer(mux) defer srv.Close() - srvURL, err := url.Parse(srv.URL) - if err != nil { - t.Fatalf("parse server url: %v", err) - } - r, err := NewResolverWithCacheDir(t.TempDir()) if err != nil { t.Fatalf("new resolver: %v", err) } - r.httpClient = &http.Client{Transport: &redirectTransport{target: srvURL}} + r.apiBaseURL = srv.URL cfg := PackConfig{Name: packName, Source: "github.com/org/repo", Version: version} @@ -108,19 +97,46 @@ func TestResolveConcurrentSamePackDownloadsOnce(t *testing.T) { } } +// writeReleaseJSON writes a GitHub Releases API response whose asset download +// URLs point back at baseURL (so the resolver's default HTTP client reaches the +// stub without any transport rewriting). +func writeReleaseJSON(w http.ResponseWriter, baseURL, tag string, assets []githubAsset) { + for i := range assets { + if assets[i].BrowserDownloadURL == "" { + assets[i].BrowserDownloadURL = fmt.Sprintf("%s/dl/%s/%s", baseURL, tag, assets[i].Name) + } + } + _ = json.NewEncoder(w).Encode(githubRelease{TagName: tag, Assets: assets}) +} + // buildTarGz returns a .tar.gz archive containing a single entry named // binaryName with the given content. func buildTarGz(t *testing.T, binaryName string, content []byte) []byte { + t.Helper() + return buildTarGzMulti(t, []tarEntry{{name: binaryName, content: content, mode: 0755}}) +} + +// tarEntry describes a single file written into a test archive. +type tarEntry struct { + name string + content []byte + mode int64 +} + +// buildTarGzMulti returns a .tar.gz archive containing the given entries. +func buildTarGzMulti(t *testing.T, entries []tarEntry) []byte { t.Helper() var buf bytes.Buffer gz := gzip.NewWriter(&buf) tw := tar.NewWriter(gz) - hdr := &tar.Header{Name: binaryName, Mode: 0755, Size: int64(len(content))} - if err := tw.WriteHeader(hdr); err != nil { - t.Fatalf("write tar header: %v", err) - } - if _, err := tw.Write(content); err != nil { - t.Fatalf("write tar content: %v", err) + for _, e := range entries { + hdr := &tar.Header{Name: e.name, Mode: e.mode, Size: int64(len(e.content)), Typeflag: tar.TypeReg} + if err := tw.WriteHeader(hdr); err != nil { + t.Fatalf("write tar header: %v", err) + } + if _, err := tw.Write(e.content); err != nil { + t.Fatalf("write tar content: %v", err) + } } if err := tw.Close(); err != nil { t.Fatalf("close tar: %v", err) diff --git a/internal/packs/resolver/resolver_test.go b/internal/packs/resolver/resolver_test.go new file mode 100644 index 0000000..51b6405 --- /dev/null +++ b/internal/packs/resolver/resolver_test.go @@ -0,0 +1,330 @@ +package resolver + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// platformArchiveName builds a goreleaser-style archive name for the test +// platform. +func platformArchiveName(prefix, version string) string { + return fmt.Sprintf("%s_%s_%s_%s.tar.gz", prefix, version, runtime.GOOS, runtime.GOARCH) +} + +// stubConfig configures the fake GitHub Releases server. +type stubConfig struct { + latestTag string // tag returned for /releases/latest + releases map[string]bool // tags that exist (e.g. "v1.2.3") + assets []githubAsset // asset list returned for any existing release + downloads map[string][]byte // asset name -> body served on download + checksumsBody string // body served for checksums.txt +} + +// startStub starts a server emulating the GitHub Releases API + asset downloads +// for a single repo. Asset download URLs point back at the server. +func startStub(t *testing.T, cfg stubConfig) *httptest.Server { + t.Helper() + var srv *httptest.Server + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + switch { + case strings.HasSuffix(path, "/releases/latest"): + if cfg.latestTag == "" { + http.NotFound(w, r) + return + } + writeReleaseJSON(w, srv.URL, cfg.latestTag, cfg.assets) + case strings.Contains(path, "/releases/tags/"): + tag := path[strings.LastIndex(path, "/")+1:] + if !cfg.releases[tag] { + http.NotFound(w, r) + return + } + writeReleaseJSON(w, srv.URL, tag, cfg.assets) + case strings.HasSuffix(path, "/checksums.txt"): + _, _ = w.Write([]byte(cfg.checksumsBody)) + case strings.HasPrefix(path, "/dl/"): + name := path[strings.LastIndex(path, "/")+1:] + body, ok := cfg.downloads[name] + if !ok { + http.NotFound(w, r) + return + } + _, _ = w.Write(body) + default: + http.NotFound(w, r) + } + }) + srv = httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +func checksumsFor(names map[string][]byte) string { + var b strings.Builder + for name, body := range names { + sum := sha256.Sum256(body) + fmt.Fprintf(&b, "%s %s\n", hex.EncodeToString(sum[:]), name) + } + return b.String() +} + +func newTestResolver(t *testing.T, apiBaseURL string) *Resolver { + t.Helper() + r, err := NewResolverWithCacheDir(t.TempDir()) + if err != nil { + t.Fatalf("new resolver: %v", err) + } + r.apiBaseURL = apiBaseURL + return r +} + +// TestResolve_EmptyVersionRejected verifies runtime resolution requires a +// pinned version: a row with an empty version (e.g. legacy data) must fail +// loudly rather than fetch "latest" into the pack's root cache dir, where it +// would sit next to version subdirectories and be picked up by cache scans. +func TestResolve_EmptyVersionRejected(t *testing.T) { + r := newTestResolver(t, "http://unused.invalid") + + _, err := r.Resolve(context.Background(), PackConfig{Name: "mypack", Source: "github.com/org/repo"}) + if err == nil || !strings.Contains(err.Error(), "version is required") { + t.Fatalf("err = %v, want version-required error", err) + } +} + +// TestFetch_PinnedVersion resolves an explicit version tag, verifies the +// checksum, and extracts the binary named after the asset prefix. +func TestFetch_PinnedVersion(t *testing.T) { + const prefix, version = "mypack", "1.2.3" + archiveName := platformArchiveName(prefix, version) + binaryContent := []byte("#!/bin/sh\necho hi\n") + archive := buildTarGz(t, prefix, binaryContent) + + srv := startStub(t, stubConfig{ + releases: map[string]bool{"v" + version: true}, + assets: []githubAsset{{Name: archiveName}, {Name: "checksums.txt"}}, + downloads: map[string][]byte{archiveName: archive}, + checksumsBody: checksumsFor(map[string][]byte{archiveName: archive}), + }) + r := newTestResolver(t, srv.URL) + + dest := t.TempDir() + res, err := r.Fetch(context.Background(), "github.com/org/repo", version, dest) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + if res.Version != version { + t.Fatalf("Version = %q, want %q", res.Version, version) + } + if filepath.Base(res.BinaryPath) != prefix { + t.Fatalf("binary base = %q, want %q", filepath.Base(res.BinaryPath), prefix) + } + got, err := os.ReadFile(res.BinaryPath) + if err != nil { + t.Fatalf("read binary: %v", err) + } + if string(got) != string(binaryContent) { + t.Fatalf("binary content mismatch") + } +} + +// TestFetch_VPrefixedInputEquivalent verifies that "v1.2.3" and "1.2.3" both +// resolve, so an operator can paste the tag exactly as GitHub displays it. +func TestFetch_VPrefixedInputEquivalent(t *testing.T) { + const prefix, version = "mypack", "1.2.3" + archiveName := platformArchiveName(prefix, version) + archive := buildTarGz(t, prefix, []byte("bin")) + stub := stubConfig{ + releases: map[string]bool{"v" + version: true}, + assets: []githubAsset{{Name: archiveName}, {Name: "checksums.txt"}}, + downloads: map[string][]byte{archiveName: archive}, + checksumsBody: checksumsFor(map[string][]byte{archiveName: archive}), + } + + for _, input := range []string{"1.2.3", "v1.2.3"} { + srv := startStub(t, stub) + r := newTestResolver(t, srv.URL) + res, err := r.Fetch(context.Background(), "github.com/org/repo", input, t.TempDir()) + if err != nil { + t.Fatalf("Fetch(%q): %v", input, err) + } + if res.Version != version { + t.Fatalf("Fetch(%q) Version = %q, want %q", input, res.Version, version) + } + } +} + +// TestFetch_BareTagFallback resolves a repo whose tags are not v-prefixed by +// falling back to the bare tag after the v-prefixed lookup 404s. +func TestFetch_BareTagFallback(t *testing.T) { + const prefix, version = "mypack", "1.2.3" + archiveName := platformArchiveName(prefix, version) + archive := buildTarGz(t, prefix, []byte("bin")) + srv := startStub(t, stubConfig{ + releases: map[string]bool{version: true}, // tag "1.2.3", no leading v + assets: []githubAsset{{Name: archiveName}, {Name: "checksums.txt"}}, + downloads: map[string][]byte{archiveName: archive}, + checksumsBody: checksumsFor(map[string][]byte{archiveName: archive}), + }) + r := newTestResolver(t, srv.URL) + + res, err := r.Fetch(context.Background(), "github.com/org/repo", "v1.2.3", t.TempDir()) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + if res.Version != version { + t.Fatalf("Version = %q, want %q", res.Version, version) + } +} + +// TestFetch_LatestResolution resolves an empty version through /releases/latest +// and pins the concrete tag (leading "v" stripped). +func TestFetch_LatestResolution(t *testing.T) { + const prefix, version = "mypack", "9.9.9" + archiveName := platformArchiveName(prefix, version) + archive := buildTarGz(t, prefix, []byte("bin")) + + srv := startStub(t, stubConfig{ + latestTag: "v" + version, + releases: map[string]bool{"v" + version: true}, + assets: []githubAsset{{Name: archiveName}, {Name: "checksums.txt"}}, + downloads: map[string][]byte{archiveName: archive}, + checksumsBody: checksumsFor(map[string][]byte{archiveName: archive}), + }) + r := newTestResolver(t, srv.URL) + + res, err := r.Fetch(context.Background(), "github.com/org/repo", "", t.TempDir()) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + if res.Version != version { + t.Fatalf("Version = %q, want %q (concrete tag, v stripped)", res.Version, version) + } +} + +// TestFetch_NoMatchingAsset fails when no asset matches the platform suffix. +func TestFetch_NoMatchingAsset(t *testing.T) { + srv := startStub(t, stubConfig{ + releases: map[string]bool{"v1.0.0": true}, + assets: []githubAsset{{Name: "mypack_1.0.0_plan9_sparc.tar.gz"}, {Name: "checksums.txt"}}, + }) + r := newTestResolver(t, srv.URL) + + _, err := r.Fetch(context.Background(), "github.com/org/repo", "1.0.0", t.TempDir()) + if err == nil || !strings.Contains(err.Error(), "no asset matching") { + t.Fatalf("expected no-asset error, got %v", err) + } +} + +// TestFetch_MultipleMatchingAssets fails and lists the candidates. +func TestFetch_MultipleMatchingAssets(t *testing.T) { + a1 := platformArchiveName("mypack", "1.0.0") + a2 := platformArchiveName("other", "1.0.0") + srv := startStub(t, stubConfig{ + releases: map[string]bool{"v1.0.0": true}, + assets: []githubAsset{{Name: a1}, {Name: a2}, {Name: "checksums.txt"}}, + }) + r := newTestResolver(t, srv.URL) + + _, err := r.Fetch(context.Background(), "github.com/org/repo", "1.0.0", t.TempDir()) + if err == nil || !strings.Contains(err.Error(), "multiple assets match") { + t.Fatalf("expected multiple-asset error, got %v", err) + } + if !strings.Contains(err.Error(), a1) || !strings.Contains(err.Error(), a2) { + t.Fatalf("error should list candidates, got %v", err) + } +} + +// TestFetch_ChecksumMismatch fails when the archive does not match checksums.txt. +func TestFetch_ChecksumMismatch(t *testing.T) { + const prefix, version = "mypack", "1.0.0" + archiveName := platformArchiveName(prefix, version) + archive := buildTarGz(t, prefix, []byte("real")) + + srv := startStub(t, stubConfig{ + releases: map[string]bool{"v" + version: true}, + assets: []githubAsset{{Name: archiveName}, {Name: "checksums.txt"}}, + downloads: map[string][]byte{archiveName: archive}, + checksumsBody: checksumsFor(map[string][]byte{archiveName: []byte("different bytes")}), + }) + r := newTestResolver(t, srv.URL) + + _, err := r.Fetch(context.Background(), "github.com/org/repo", version, t.TempDir()) + if err == nil || !strings.Contains(err.Error(), "checksum mismatch") { + t.Fatalf("expected checksum mismatch, got %v", err) + } +} + +// TestFetch_RepoOrTagNotFound maps a 404 to a clear error. +func TestFetch_RepoOrTagNotFound(t *testing.T) { + srv := startStub(t, stubConfig{releases: map[string]bool{}}) + r := newTestResolver(t, srv.URL) + + _, err := r.Fetch(context.Background(), "github.com/org/repo", "1.0.0", t.TempDir()) + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Fatalf("expected not-found error, got %v", err) + } +} + +// TestFetch_SingleExecutableFallback extracts the sole executable when no entry +// matches the asset prefix. +func TestFetch_SingleExecutableFallback(t *testing.T) { + const version = "1.0.0" + // Asset prefix is "weird" but the binary inside is named "actualbin"; a + // non-executable README is also present. + archiveName := platformArchiveName("weird", version) + archive := buildTarGzMulti(t, []tarEntry{ + {name: "README.md", content: []byte("readme"), mode: 0644}, + {name: "actualbin", content: []byte("the binary"), mode: 0755}, + }) + + srv := startStub(t, stubConfig{ + releases: map[string]bool{"v" + version: true}, + assets: []githubAsset{{Name: archiveName}, {Name: "checksums.txt"}}, + downloads: map[string][]byte{archiveName: archive}, + checksumsBody: checksumsFor(map[string][]byte{archiveName: archive}), + }) + r := newTestResolver(t, srv.URL) + + res, err := r.Fetch(context.Background(), "github.com/org/repo", version, t.TempDir()) + if err != nil { + t.Fatalf("Fetch: %v", err) + } + if filepath.Base(res.BinaryPath) != "actualbin" { + t.Fatalf("binary base = %q, want fallback to single executable %q", filepath.Base(res.BinaryPath), "actualbin") + } +} + +// TestFetch_AmbiguousBinaryFails errors when neither the prefix matches nor a +// single executable can be identified. +func TestFetch_AmbiguousBinaryFails(t *testing.T) { + const version = "1.0.0" + archiveName := platformArchiveName("weird", version) + archive := buildTarGzMulti(t, []tarEntry{ + {name: "binA", content: []byte("a"), mode: 0755}, + {name: "binB", content: []byte("b"), mode: 0755}, + }) + + srv := startStub(t, stubConfig{ + releases: map[string]bool{"v" + version: true}, + assets: []githubAsset{{Name: archiveName}, {Name: "checksums.txt"}}, + downloads: map[string][]byte{archiveName: archive}, + checksumsBody: checksumsFor(map[string][]byte{archiveName: archive}), + }) + r := newTestResolver(t, srv.URL) + + _, err := r.Fetch(context.Background(), "github.com/org/repo", version, t.TempDir()) + if err == nil || !strings.Contains(err.Error(), "cannot determine pack binary") { + t.Fatalf("expected ambiguous-binary error, got %v", err) + } +} diff --git a/internal/testutil/fakes/fakes.go b/internal/testutil/fakes/fakes.go index 23bfea8..70e522d 100644 --- a/internal/testutil/fakes/fakes.go +++ b/internal/testutil/fakes/fakes.go @@ -784,6 +784,12 @@ func (s *ConfigStore) GetAppConfig(_ context.Context) (config.AppConfig, error) out.RunRetentionDays = v } } + if raw, ok := s.data["default_tags"]; ok { + var v map[string]string + if err := json.Unmarshal(raw, &v); err == nil && v != nil { + out.DefaultTags = v + } + } return out, nil } @@ -799,6 +805,7 @@ func (s *ConfigStore) UpdateAppConfig(_ context.Context, c config.AppConfig) err "run_log_retention_days": c.RunLogRetentionDays, "run_retention_enabled": c.RunRetentionEnabled, "run_retention_days": c.RunRetentionDays, + "default_tags": c.DefaultTags, } { raw, err := json.Marshal(v) if err != nil { diff --git a/internal/web/api_config_test.go b/internal/web/api_config_test.go index c2ffb92..99a6324 100644 --- a/internal/web/api_config_test.go +++ b/internal/web/api_config_test.go @@ -49,3 +49,47 @@ func TestHandleUpdateConfig_RetentionDaysPersistsValid(t *testing.T) { require.NoError(t, err) assert.Equal(t, 14, cfg.RunRetentionDays) } + +// default_tags must be a string→string object because it is merged per-key +// into pack parameters; anything else would be silently skipped downstream. +func TestHandleUpdateConfig_DefaultTagsRejectsInvalid(t *testing.T) { + for name, value := range map[string]string{ + "non-object": `"owner=secops"`, + "non-string value": `{"owner": 123}`, + "null": `null`, + } { + t.Run(name, func(t *testing.T) { + ts := testserver.New(t) + + before, err := ts.Stores.Config.GetAppConfig(t.Context()) + require.NoError(t, err) + + resp := ts.Put(t, "/api/config", web.UpdateConfigRequest{ + Key: "default_tags", + Value: []byte(value), + }) + defer resp.Body.Close() + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + + // The rejected write must leave the stored config untouched. + after, err := ts.Stores.Config.GetAppConfig(t.Context()) + require.NoError(t, err) + assert.Equal(t, before, after) + }) + } +} + +func TestHandleUpdateConfig_DefaultTagsPersistsValid(t *testing.T) { + ts := testserver.New(t) + + resp := ts.Put(t, "/api/config", web.UpdateConfigRequest{ + Key: "default_tags", + Value: []byte(`{"owner": "secops", "simulated": "true"}`), + }) + defer resp.Body.Close() + require.Equal(t, http.StatusNoContent, resp.StatusCode) + + cfg, err := ts.Stores.Config.GetAppConfig(t.Context()) + require.NoError(t, err) + assert.Equal(t, map[string]string{"owner": "secops", "simulated": "true"}, cfg.DefaultTags) +} diff --git a/internal/web/api_packs_install_test.go b/internal/web/api_packs_install_test.go new file mode 100644 index 0000000..edc7fd6 --- /dev/null +++ b/internal/web/api_packs_install_test.go @@ -0,0 +1,256 @@ +package web_test + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/IBM/simrun/internal/testutil/testserver" + "github.com/IBM/simrun/internal/web" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// manifestScript returns the bytes of a shell script that prints a manifest +// response reporting the given pack name and version. +func manifestScript(name, version string) []byte { + return fmt.Appendf(nil, "#!/bin/sh\necho '{\"pack\":{\"name\":\"%s\",\"version\":\"%s\"},\"simulations\":[]}'\n", name, version) +} + +// failingManifestScript returns a script whose manifest command exits non-zero. +func failingManifestScript() []byte { + return []byte("#!/bin/sh\necho boom >&2\nexit 1\n") +} + +// writeExecutable writes content as an executable file under dir and returns its path. +func writeExecutable(t *testing.T, dir, name string, content []byte) string { + t.Helper() + p := filepath.Join(dir, name) + require.NoError(t, os.WriteFile(p, content, 0755)) + return p +} + +// platformArchiveName builds a goreleaser-style archive name for the test platform. +func platformArchiveName(prefix, version string) string { + return fmt.Sprintf("%s_%s_%s_%s.tar.gz", prefix, version, runtime.GOOS, runtime.GOARCH) +} + +// tarGzWith builds a .tar.gz containing a single executable entry. +func tarGzWith(t *testing.T, binaryName string, content []byte) []byte { + t.Helper() + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + tw := tar.NewWriter(gz) + require.NoError(t, tw.WriteHeader(&tar.Header{Name: binaryName, Mode: 0755, Size: int64(len(content)), Typeflag: tar.TypeReg})) + _, err := tw.Write(content) + require.NoError(t, err) + require.NoError(t, tw.Close()) + require.NoError(t, gz.Close()) + return buf.Bytes() +} + +type releaseJSON struct { + TagName string `json:"tag_name"` + Assets []assetJSON `json:"assets"` +} + +type assetJSON struct { + Name string `json:"name"` + BrowserDownloadURL string `json:"browser_download_url"` +} + +// startGitHubStub emulates the GitHub Releases API + downloads for a single +// repo with one platform archive. latestTag is returned for /releases/latest; +// existingTags are the tags resolvable via /releases/tags/. +func startGitHubStub(t *testing.T, latestTag string, existingTags map[string]bool, archiveName string, archive []byte) *httptest.Server { + t.Helper() + checksums := func() string { + sum := sha256.Sum256(archive) + return fmt.Sprintf("%s %s\n", hex.EncodeToString(sum[:]), archiveName) + }() + + var srv *httptest.Server + mux := http.NewServeMux() + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + writeRel := func(tag string) { + _ = json.NewEncoder(w).Encode(releaseJSON{ + TagName: tag, + Assets: []assetJSON{ + {Name: archiveName, BrowserDownloadURL: srv.URL + "/dl/" + archiveName}, + {Name: "checksums.txt", BrowserDownloadURL: srv.URL + "/dl/checksums.txt"}, + }, + }) + } + switch { + case strings.HasSuffix(path, "/releases/latest"): + if latestTag == "" { + http.NotFound(w, r) + return + } + writeRel(latestTag) + case strings.Contains(path, "/releases/tags/"): + tag := path[strings.LastIndex(path, "/")+1:] + if !existingTags[tag] { + http.NotFound(w, r) + return + } + writeRel(tag) + case strings.HasSuffix(path, "/dl/checksums.txt"): + _, _ = w.Write([]byte(checksums)) + case strings.HasSuffix(path, "/dl/"+archiveName): + _, _ = w.Write(archive) + default: + http.NotFound(w, r) + } + }) + srv = httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +// TestInstallPack_RemotePinsResolvedTagAndManifestName verifies a remote install +// downloads, runs the manifest, ignores the request name, and pins the row's +// version to the resolved release tag. +func TestInstallPack_RemotePinsResolvedTagAndManifestName(t *testing.T) { + const tag, version = "v2.3.4", "2.3.4" + archiveName := platformArchiveName("realpack", version) + // The in-tarball binary is named "realpack" but the manifest reports a + // different identity: "real-pack". The row must take the manifest name. + archive := tarGzWith(t, "realpack", manifestScript("real-pack", "manifest-version-ignored")) + srv := startGitHubStub(t, "", map[string]bool{tag: true}, archiveName, archive) + t.Setenv("SR_GITHUB_API_URL", srv.URL) + + ts := testserver.New(t) + resp := ts.Post(t, "/api/packs/install", web.InstallPackRequest{ + Name: "operator-typed", + Type: "remote", + Source: "github.com/org/repo", + Version: version, + }) + defer resp.Body.Close() + require.Equal(t, http.StatusCreated, resp.StatusCode, testserver.ReadBody(t, resp)) + + got, err := ts.Stores.Pack.Get(t.Context(), "real-pack") + require.NoError(t, err, "row must be keyed by the manifest name, not the request name") + assert.Equal(t, "2.3.4", got.Version, "version must be the resolved release tag") + assert.Equal(t, "github.com/org/repo", got.Source) + + _, err = ts.Stores.Pack.Get(t.Context(), "operator-typed") + assert.Error(t, err, "request name must be ignored") +} + +// TestInstallPack_RemoteLatest resolves an omitted version to the latest tag. +func TestInstallPack_RemoteLatest(t *testing.T) { + const tag, version = "v9.9.9", "9.9.9" + archiveName := platformArchiveName("realpack", version) + archive := tarGzWith(t, "realpack", manifestScript("latest-pack", "ignored")) + srv := startGitHubStub(t, tag, map[string]bool{tag: true}, archiveName, archive) + t.Setenv("SR_GITHUB_API_URL", srv.URL) + + ts := testserver.New(t) + resp := ts.Post(t, "/api/packs/install", web.InstallPackRequest{ + Type: "remote", + Source: "github.com/org/repo", + }) + defer resp.Body.Close() + require.Equal(t, http.StatusCreated, resp.StatusCode, testserver.ReadBody(t, resp)) + + got, err := ts.Stores.Pack.Get(t.Context(), "latest-pack") + require.NoError(t, err) + assert.Equal(t, "9.9.9", got.Version, "latest must be pinned to the concrete tag") +} + +// TestInstallPack_RemoteRepoNotFound creates no row when the release is missing. +func TestInstallPack_RemoteRepoNotFound(t *testing.T) { + srv := startGitHubStub(t, "", map[string]bool{}, "x.tar.gz", nil) + t.Setenv("SR_GITHUB_API_URL", srv.URL) + + ts := testserver.New(t) + resp := ts.Post(t, "/api/packs/install", web.InstallPackRequest{ + Type: "remote", + Source: "github.com/org/repo", + Version: "1.0.0", + }) + defer resp.Body.Close() + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + + packs, err := ts.Stores.Pack.List(t.Context()) + require.NoError(t, err) + assert.Empty(t, packs, "no DB row on resolution failure") +} + +// TestInstallPack_LocalManifestDerived takes name + version from the manifest. +func TestInstallPack_LocalManifestDerived(t *testing.T) { + bin := writeExecutable(t, t.TempDir(), "anything", manifestScript("local-pack", "0.7.0")) + + ts := testserver.New(t) + resp := ts.Post(t, "/api/packs/install", web.InstallPackRequest{ + Name: "ignored", + Type: "local", + Source: bin, + }) + defer resp.Body.Close() + require.Equal(t, http.StatusCreated, resp.StatusCode, testserver.ReadBody(t, resp)) + + got, err := ts.Stores.Pack.Get(t.Context(), "local-pack") + require.NoError(t, err) + assert.Equal(t, "0.7.0", got.Version, "local version comes from the manifest") + assert.Equal(t, bin, got.Source, "local source is referenced in place") +} + +// TestInstallPack_LocalNonExistentPath rejects a missing path with no row. +func TestInstallPack_LocalNonExistentPath(t *testing.T) { + ts := testserver.New(t) + resp := ts.Post(t, "/api/packs/install", web.InstallPackRequest{ + Type: "local", + Source: filepath.Join(t.TempDir(), "does-not-exist"), + }) + defer resp.Body.Close() + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + + packs, err := ts.Stores.Pack.List(t.Context()) + require.NoError(t, err) + assert.Empty(t, packs, "no DB row when the path does not exist") +} + +// TestInstallPack_LocalManifestFailure rejects a binary whose manifest fails. +func TestInstallPack_LocalManifestFailure(t *testing.T) { + bin := writeExecutable(t, t.TempDir(), "broken", failingManifestScript()) + + ts := testserver.New(t) + resp := ts.Post(t, "/api/packs/install", web.InstallPackRequest{ + Type: "local", + Source: bin, + }) + defer resp.Body.Close() + require.Equal(t, http.StatusBadRequest, resp.StatusCode) + + packs, err := ts.Stores.Pack.List(t.Context()) + require.NoError(t, err) + assert.Empty(t, packs, "no DB row when the manifest command fails") +} + +// TestInstallPack_UploadTypeRejectedOnInstall directs upload installs to the +// dedicated endpoint. +func TestInstallPack_UploadTypeRejectedOnInstall(t *testing.T) { + ts := testserver.New(t) + resp := ts.Post(t, "/api/packs/install", web.InstallPackRequest{Type: "upload", Source: "x"}) + defer resp.Body.Close() + assert.Equal(t, http.StatusBadRequest, resp.StatusCode) + + packs, err := ts.Stores.Pack.List(t.Context()) + require.NoError(t, err) + assert.Empty(t, packs) +} diff --git a/internal/web/api_packs_test.go b/internal/web/api_packs_test.go index 36a93e4..e94263d 100644 --- a/internal/web/api_packs_test.go +++ b/internal/web/api_packs_test.go @@ -26,21 +26,23 @@ func TestHandleListPacks_Empty(t *testing.T) { func TestHandleInstallPack_AndList(t *testing.T) { ts := testserver.New(t) + // Install is now eager: a local install verifies the path and runs the + // pack's manifest to derive the identity. The request name is ignored. + bin := writeExecutable(t, t.TempDir(), "anything", manifestScript("base-pack", "1.0.0")) resp := ts.Post(t, "/api/packs/install", web.InstallPackRequest{ - Name: "base", - Type: "remote", - Source: "owner/repo", - Version: "v1.0.0", + Name: "ignored", + Type: "local", + Source: bin, }) defer resp.Body.Close() - require.Equal(t, http.StatusCreated, resp.StatusCode) + require.Equal(t, http.StatusCreated, resp.StatusCode, testserver.ReadBody(t, resp)) resp = ts.Get(t, "/api/packs") defer resp.Body.Close() var packs []db.Pack testserver.DecodeJSON(t, resp, &packs) require.Len(t, packs, 1) - assert.Equal(t, "base", packs[0].Name) + assert.Equal(t, "base-pack", packs[0].Name) assert.Equal(t, "installed", packs[0].Status) } diff --git a/internal/web/handlers.go b/internal/web/handlers.go index cfdce09..875780a 100644 --- a/internal/web/handlers.go +++ b/internal/web/handlers.go @@ -489,6 +489,16 @@ func (h *Handlers) HandleUpdateConfig(w http.ResponseWriter, r *http.Request) { } } + // default_tags feeds the per-pack merge, so it must be a string→string + // object or the merge would silently skip it. + if req.Key == "default_tags" { + var tags map[string]string + if err := json.Unmarshal(req.Value, &tags); err != nil || tags == nil { + writeError(w, http.StatusBadRequest, "default_tags must be an object with string values") + return + } + } + if err := h.configStore.Set(r.Context(), req.Key, req.Value); err != nil { writeError(w, http.StatusInternalServerError, err.Error()) return diff --git a/internal/web/packs_handler.go b/internal/web/packs_handler.go index 5cbf2f5..eeb72e1 100644 --- a/internal/web/packs_handler.go +++ b/internal/web/packs_handler.go @@ -1,6 +1,7 @@ package web import ( + "context" "encoding/json" "fmt" "io" @@ -12,6 +13,7 @@ import ( "github.com/IBM/simrun/internal/config" "github.com/IBM/simrun/internal/db" "github.com/IBM/simrun/internal/packs/locks" + "github.com/IBM/simrun/internal/packs/resolver" packrunner "github.com/IBM/simrun/internal/packs/runner" "github.com/go-chi/chi/v5" ) @@ -37,7 +39,14 @@ func (h *PackHandlers) HandleListPacks(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, packs) } -// HandleInstallPack handles POST /api/packs/install +// HandleInstallPack handles POST /api/packs/install. +// +// Install is an eager operation: the pack binary is made available (downloaded +// and checksum-verified for remote, verified on disk for local), its manifest +// command is run to derive the pack's identity, and only then is a row +// persisted. Any failure (bad repo, missing asset, checksum mismatch, manifest +// error, non-existent path) returns an error and creates no DB record. The +// request's `name` is ignored — the manifest is the source of truth. func (h *PackHandlers) HandleInstallPack(w http.ResponseWriter, r *http.Request) { var req InstallPackRequest if err := json.NewDecoder(r.Body).Decode(&req); err != nil { @@ -46,21 +55,32 @@ func (h *PackHandlers) HandleInstallPack(w http.ResponseWriter, r *http.Request) } switch config.PackType(req.Type) { - case config.PackTypeLocal, config.PackTypeRemote, config.PackTypeUpload: - // recognized + case config.PackTypeLocal, config.PackTypeRemote: + // installed below + case config.PackTypeUpload: + writeError(w, http.StatusBadRequest, "upload packs must be installed via POST /api/packs/upload") + return default: writeError(w, http.StatusBadRequest, fmt.Sprintf("invalid pack type %q: allowed types are local, remote, upload", req.Type)) return } - pack := &db.Pack{ - Name: req.Name, - Type: req.Type, - Source: req.Source, - Version: req.Version, - Status: "installed", - Parameters: req.Parameters, + res, err := resolver.NewResolver(h.dataDir) + if err != nil { + writeError(w, http.StatusInternalServerError, "failed to create pack resolver") + return + } + + var pack *db.Pack + if config.PackType(req.Type) == config.PackTypeRemote { + pack, err = h.installRemote(r.Context(), res, req) + } else { + pack, err = h.installLocal(r.Context(), res, req) + } + if err != nil { + writeError(w, http.StatusBadRequest, err.Error()) + return } if err := h.packStore.Upsert(r.Context(), pack, getUserEmail(r)); err != nil { @@ -71,6 +91,114 @@ func (h *PackHandlers) HandleInstallPack(w http.ResponseWriter, r *http.Request) writeJSON(w, http.StatusCreated, pack) } +// installRemote downloads + verifies + extracts a remote pack into a staging +// dir, runs its manifest to derive the name, then relocates the binary to the +// canonical version-keyed cache path. The persisted version is the resolved +// release tag (not an operator-typed value). +func (h *PackHandlers) installRemote(ctx context.Context, res *resolver.Resolver, req InstallPackRequest) (*db.Pack, error) { + if req.Source == "" { + return nil, fmt.Errorf("source is required for remote packs") + } + + // Stage the download inside the cache dir so the later relocate is a + // same-filesystem rename. + staging, err := os.MkdirTemp(res.CacheDir(), ".staging-") + if err != nil { + return nil, fmt.Errorf("failed to create staging directory: %w", err) + } + defer func() { _ = os.RemoveAll(staging) }() + + fetched, err := res.Fetch(ctx, req.Source, req.Version, staging) + if err != nil { + return nil, err + } + + manifest, err := res.GetManifest(ctx, fetched.BinaryPath) + if err != nil { + return nil, fmt.Errorf("manifest validation failed: %w", err) + } + name := manifest.Pack.Name + if err := validateManifestName(name); err != nil { + return nil, err + } + + // Relocate under the per-pack lock so a concurrent install/delete of the + // same pack cannot interleave with the rename. + release := locks.Acquire(name) + defer release() + + destPath := filepath.Join(res.CacheDir(), name, fetched.Version, filepath.Base(fetched.BinaryPath)) + if err := moveFile(fetched.BinaryPath, destPath); err != nil { + return nil, err + } + + return &db.Pack{ + Name: name, + Type: string(config.PackTypeRemote), + Source: req.Source, + Version: fetched.Version, + Status: "installed", + Parameters: req.Parameters, + }, nil +} + +// installLocal verifies the source path exists and runs its manifest to derive +// the pack's identity. The binary is referenced in place, never copied. +func (h *PackHandlers) installLocal(ctx context.Context, res *resolver.Resolver, req InstallPackRequest) (*db.Pack, error) { + if req.Source == "" { + return nil, fmt.Errorf("source (path) is required for local packs") + } + if _, err := os.Stat(req.Source); err != nil { + return nil, fmt.Errorf("pack binary not found at %s: %w", req.Source, err) + } + + manifest, err := res.GetManifest(ctx, req.Source) + if err != nil { + return nil, fmt.Errorf("manifest validation failed: %w", err) + } + name := manifest.Pack.Name + if err := validateManifestName(name); err != nil { + return nil, err + } + + return &db.Pack{ + Name: name, + Type: string(config.PackTypeLocal), + Source: req.Source, + Version: manifest.Pack.Version, + Status: "installed", + Parameters: req.Parameters, + }, nil +} + +// validateManifestName rejects manifest names that are unsafe to use as a +// filesystem path component (the name becomes a cache directory for remote and +// upload packs). +func validateManifestName(name string) error { + if name == "" { + return fmt.Errorf("pack manifest did not report a name") + } + if strings.Contains(name, "..") || strings.ContainsAny(name, "/\\") || name != filepath.Base(name) { + return fmt.Errorf("invalid pack name %q from manifest: cannot contain path separators or '..'", name) + } + return nil +} + +// moveFile relocates srcPath to destPath, creating the destination directory +// and replacing any existing file. Used to promote a staged binary to its +// canonical cache path; staging dirs live under the cache dir so this is a +// same-filesystem rename. +func moveFile(srcPath, destPath string) error { + if err := os.MkdirAll(filepath.Dir(destPath), 0755); err != nil { + return fmt.Errorf("failed to create cache directory: %w", err) + } + _ = os.Remove(destPath) + if err := os.Rename(srcPath, destPath); err != nil { + return fmt.Errorf("failed to relocate pack binary: %w", err) + } + return nil +} + // HandleDeletePack handles DELETE /api/packs/{name} func (h *PackHandlers) HandleDeletePack(w http.ResponseWriter, r *http.Request) { name := chi.URLParam(r, "name") @@ -227,7 +355,12 @@ func (h *PackHandlers) fetchPackParamsSchema(r *http.Request, pack *db.Pack) *pa // maxUploadSize is the maximum allowed pack binary upload size (500MB). const maxUploadSize = 500 << 20 -// HandleUploadPack handles POST /api/packs/upload +// HandleUploadPack handles POST /api/packs/upload. +// +// The uploaded binary is written to a staging location, its manifest command is +// run to derive the pack's identity, then it is relocated to +// /packs//upload/. The request's `name` form field is +// ignored. A manifest failure aborts the install with no DB record. func (h *PackHandlers) HandleUploadPack(w http.ResponseWriter, r *http.Request) { r.Body = http.MaxBytesReader(w, r.Body, maxUploadSize) @@ -236,18 +369,6 @@ func (h *PackHandlers) HandleUploadPack(w http.ResponseWriter, r *http.Request) return } - name := r.FormValue("name") - if name == "" { - writeError(w, http.StatusBadRequest, "Pack name is required") - return - } - - // Validate name to prevent directory traversal - if strings.Contains(name, "..") || strings.ContainsAny(name, "/\\") || name != filepath.Base(name) { - writeError(w, http.StatusBadRequest, "Invalid pack name: cannot contain path separators or '..'") - return - } - file, _, err := r.FormFile("file") if err != nil { writeError(w, http.StatusBadRequest, "File upload is required") @@ -255,70 +376,76 @@ func (h *PackHandlers) HandleUploadPack(w http.ResponseWriter, r *http.Request) } defer file.Close() - // Serialize the write + manifest-validate + upsert against any concurrent - // upload or delete of the same pack so they cannot interleave mid-write. - release := locks.Acquire(name) - defer release() - - // Determine upload directory - uploadDir := filepath.Join(h.dataDir, "packs", name, "upload") - binaryPath := filepath.Join(uploadDir, name) + res, err := resolver.NewResolver(h.dataDir) + if err != nil { + writeError(w, http.StatusInternalServerError, "failed to create pack resolver") + return + } - if err := os.MkdirAll(uploadDir, 0755); err != nil { - writeError(w, http.StatusInternalServerError, "Failed to create upload directory") + // Stage the upload inside the cache dir so the later relocate is a + // same-filesystem rename. + staging, err := os.MkdirTemp(res.CacheDir(), ".staging-") + if err != nil { + writeError(w, http.StatusInternalServerError, "Failed to create staging directory") return } + defer func() { _ = os.RemoveAll(staging) }() - // Write binary to disk - outFile, err := os.OpenFile(binaryPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0755) + stagedPath := filepath.Join(staging, "pack") + outFile, err := os.OpenFile(stagedPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0755) if err != nil { writeError(w, http.StatusInternalServerError, "Failed to create binary file") return } - - if _, err := io.Copy(outFile, file); err != nil { + if _, err := io.Copy(outFile, file); err != nil { //nolint:gosec // size-capped by MaxBytesReader above outFile.Close() - _ = os.Remove(binaryPath) writeError(w, http.StatusInternalServerError, "Failed to write binary file") return } if err := outFile.Close(); err != nil { - _ = os.Remove(binaryPath) writeError(w, http.StatusInternalServerError, "Failed to finalize binary file") return } - // Validate pack by running manifest command - factory, err := packrunner.NewFactory(h.dataDir) + // Derive identity from the manifest before persisting anything. + manifest, err := res.GetManifest(r.Context(), stagedPath) if err != nil { - _ = os.Remove(binaryPath) - writeError(w, http.StatusInternalServerError, "Failed to create pack runner factory") + writeError(w, http.StatusBadRequest, + fmt.Sprintf("Invalid pack binary: manifest validation failed: %v", err)) return } - - cfg := config.PackConfig{ - Name: name, - Type: config.PackTypeUpload, - Source: binaryPath, + name := manifest.Pack.Name + if err := validateManifestName(name); err != nil { + writeError(w, http.StatusBadRequest, err.Error()) + return } - if _, err := factory.GetManifest(r.Context(), cfg, nil, nil); err != nil { - _ = os.Remove(binaryPath) - writeError(w, http.StatusBadRequest, - fmt.Sprintf("Invalid pack binary: manifest validation failed: %v", err)) + // Serialize relocate + upsert against any concurrent upload or delete of the + // same pack so they cannot interleave. + release := locks.Acquire(name) + defer release() + + // Replace any previous upload of this pack, then relocate the staged binary. + uploadDir := filepath.Join(res.CacheDir(), name, "upload") + if err := os.RemoveAll(uploadDir); err != nil { + writeError(w, http.StatusInternalServerError, "Failed to clear previous upload") + return + } + binaryPath := filepath.Join(uploadDir, name) + if err := moveFile(stagedPath, binaryPath); err != nil { + writeError(w, http.StatusInternalServerError, err.Error()) return } - // Save to database pack := &db.Pack{ - Name: name, - Type: string(config.PackTypeUpload), - Source: binaryPath, - Status: "installed", + Name: name, + Type: string(config.PackTypeUpload), + Source: binaryPath, + Version: manifest.Pack.Version, + Status: "installed", } if err := h.packStore.Upsert(r.Context(), pack, getUserEmail(r)); err != nil { - _ = os.Remove(binaryPath) writeError(w, http.StatusInternalServerError, err.Error()) return } diff --git a/internal/web/scenarios.go b/internal/web/scenarios.go index 9fc55a7..feb1d94 100644 --- a/internal/web/scenarios.go +++ b/internal/web/scenarios.go @@ -3,6 +3,7 @@ package web import ( "context" "fmt" + "maps" "path/filepath" "time" @@ -46,6 +47,8 @@ func NewScenarioService(runStore db.RunStore, assessmentStore db.AssessmentStore // loadPacksFromDB returns the pack list as []config.PackConfig sourced entirely // from the database. Replaces populatePackParameters which merged YAML+DB. +// Org-wide default tags from app config are merged per-key beneath each +// pack's own default_tags so every consumer sees effective parameters. func (s *ScenarioService) loadPacksFromDB(ctx context.Context) ([]config.PackConfig, error) { if s.packStore == nil { return nil, nil @@ -54,6 +57,14 @@ func (s *ScenarioService) loadPacksFromDB(ctx context.Context) ([]config.PackCon if err != nil { return nil, err } + orgTags := map[string]string{} + if s.configStore != nil { + appCfg, err := s.configStore.GetAppConfig(ctx) + if err != nil { + return nil, fmt.Errorf("failed to load app config: %w", err) + } + orgTags = appCfg.DefaultTags + } out := make([]config.PackConfig, 0, len(dbPacks)) for _, p := range dbPacks { out = append(out, config.PackConfig{ @@ -61,12 +72,58 @@ func (s *ScenarioService) loadPacksFromDB(ctx context.Context) ([]config.PackCon Type: config.PackType(p.Type), Source: p.Source, Version: p.Version, - Parameters: p.Parameters, + Parameters: mergeOrgDefaultTags(p.Parameters, orgTags), }) } return out, nil } +// mergeOrgDefaultTags overlays a pack's own default_tags on top of the +// org-wide map: an org tag applies unless the pack sets the same key, and a +// pack cannot delete an org tag. An empty org map returns params unchanged, +// and a malformed pack-level value (not a string→string map) passes through +// with no merge, preserving pre-org-tags behavior. +func mergeOrgDefaultTags(params map[string]any, orgTags map[string]string) map[string]any { + if len(orgTags) == 0 { + return params + } + packTags := map[string]string{} + if raw, exists := params["default_tags"]; exists { + var ok bool + if packTags, ok = asStringMap(raw); !ok { + return params + } + } + merged := make(map[string]string, len(orgTags)+len(packTags)) + maps.Copy(merged, orgTags) + maps.Copy(merged, packTags) + out := make(map[string]any, len(params)+1) + maps.Copy(out, params) + out["default_tags"] = merged + return out +} + +// asStringMap converts a JSONB-decoded parameter value into a string→string +// map, reporting false when the value is not an object of strings. +func asStringMap(v any) (map[string]string, bool) { + switch typed := v.(type) { + case map[string]string: + return typed, true + case map[string]any: + out := make(map[string]string, len(typed)) + for k, val := range typed { + s, ok := val.(string) + if !ok { + return nil, false + } + out[k] = s + } + return out, true + default: + return nil, false + } +} + // Lint parses YAML and returns a summary without executing. func (s *ScenarioService) Lint(yamlContent []byte) (*LintResponse, error) { ctx := context.Background() diff --git a/internal/web/scenarios_test.go b/internal/web/scenarios_test.go new file mode 100644 index 0000000..23a1ef6 --- /dev/null +++ b/internal/web/scenarios_test.go @@ -0,0 +1,83 @@ +package web + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// The precedence contract for a tag key is: org-wide default tag < +// pack-level default_tags entry. Packs override per-key; they cannot +// delete an org tag. +func TestMergeOrgDefaultTags_OrgTagAppliesWhenPackHasNone(t *testing.T) { + params := map[string]any{"aws_region": "us-east-1"} + got := mergeOrgDefaultTags(params, map[string]string{"owner": "secops"}) + + assert.Equal(t, map[string]any{ + "aws_region": "us-east-1", + "default_tags": map[string]string{"owner": "secops"}, + }, got) +} + +func TestMergeOrgDefaultTags_PackLevelKeyWinsPerKey(t *testing.T) { + params := map[string]any{ + "default_tags": map[string]any{"owner": "red-team"}, + } + got := mergeOrgDefaultTags(params, map[string]string{"owner": "secops", "env": "sim"}) + + assert.Equal(t, map[string]any{ + "default_tags": map[string]string{"owner": "red-team", "env": "sim"}, + }, got) +} + +// An empty org map must be a no-op so behavior is identical to before +// org-wide default tags existed: the same map, not a copy. +func TestMergeOrgDefaultTags_EmptyOrgMapPassesThrough(t *testing.T) { + params := map[string]any{ + "default_tags": map[string]any{"team": "red"}, + } + for _, orgTags := range []map[string]string{nil, {}} { + got := mergeOrgDefaultTags(params, orgTags) + assert.Equal(t, params, got) + } +} + +// A pack-level default_tags that is not a string→string map cannot be +// merged; it must pass through unmodified so terraform sees exactly what +// the pack stored (current behavior preserved). +func TestMergeOrgDefaultTags_MalformedPackValueUntouched(t *testing.T) { + for name, malformed := range map[string]any{ + "string": "owner=secops", + "array": []any{"owner"}, + "non-string value": map[string]any{"owner": 123}, + } { + t.Run(name, func(t *testing.T) { + params := map[string]any{"default_tags": malformed} + got := mergeOrgDefaultTags(params, map[string]string{"owner": "secops"}) + assert.Equal(t, map[string]any{"default_tags": malformed}, got) + }) + } +} + +// Nil pack parameters with org tags set must still produce a default_tags +// entry so packs configured with no parameters inherit org tags. +func TestMergeOrgDefaultTags_NilParamsGetOrgTags(t *testing.T) { + got := mergeOrgDefaultTags(nil, map[string]string{"owner": "secops"}) + assert.Equal(t, map[string]any{ + "default_tags": map[string]string{"owner": "secops"}, + }, got) +} + +// The merge must not mutate the pack's stored map or the org map, so a +// later reload sees the original stored parameters. +func TestMergeOrgDefaultTags_DoesNotMutateInputs(t *testing.T) { + params := map[string]any{ + "default_tags": map[string]any{"owner": "red-team"}, + } + orgTags := map[string]string{"owner": "secops", "env": "sim"} + + _ = mergeOrgDefaultTags(params, orgTags) + + assert.Equal(t, map[string]any{"default_tags": map[string]any{"owner": "red-team"}}, params) + assert.Equal(t, map[string]string{"owner": "secops", "env": "sim"}, orgTags) +} diff --git a/internal/web/types.go b/internal/web/types.go index 8a05086..2e502f8 100644 --- a/internal/web/types.go +++ b/internal/web/types.go @@ -48,7 +48,10 @@ type UpdateConfigRequest struct { } type InstallPackRequest struct { - Name string `json:"name"` + // Name is ignored: a pack's identity is derived from its manifest at + // install time. It is retained only for backward compatibility with older + // clients that still send it. + Name string `json:"name,omitempty"` Type string `json:"type"` Source string `json:"source"` Version string `json:"version,omitempty"` diff --git a/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/.openspec.yaml b/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/.openspec.yaml new file mode 100644 index 0000000..d6b53de --- /dev/null +++ b/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-06-30 diff --git a/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/design.md b/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/design.md new file mode 100644 index 0000000..3d2183b --- /dev/null +++ b/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/design.md @@ -0,0 +1,93 @@ +## Context + +Today `POST /api/packs/install` is metadata-only: it validates the type and +upserts a row. The binary is never touched until a scenario runs, when +`Factory.CreateRunner` calls `Resolver.Resolve`, which (for remote packs) +hand-builds the artifact URL from the operator-supplied `name` and `version`: + +``` +archiveName = "{name}_{version}_{GOOS}_{GOARCH}.tar.gz" +url = "https://github.com/{org}/{repo}/releases/download/v{version}/{archiveName}" +extract = file whose base name == name +cache = /packs/{name}/{version}/{name} +``` + +This couples three independent things to one `name` field and requires an exact +`version`. The pack's own `manifest` command already reports the authoritative +identity (`pack.name`, `pack.version` in `pack/protocol.go`), and the GitHub +Releases API already exposes the real asset names and the latest tag — both are +better sources of truth than operator input. + +## Goals / Non-Goals + +**Goals:** +- Operator installs a remote pack by pasting only `github.com/org/repo`, with + version optional (empty → latest). +- Pack identity (`name`, `version`) is derived from the pack's manifest, never + typed by the operator, for all three pack types. +- Install failures (bad repo, no platform asset, checksum mismatch, manifest + error) surface at install time, loudly. +- Runtime resolution is unchanged: the binary cached at install is found by the + existing `name`+`version` cache key. + +**Non-Goals:** +- Private repositories / authenticated GitHub access (no `GITHUB_TOKEN`). +- A "check for updates / re-pin to latest" lifecycle action — out of scope. +- Cleaning up stale cached binaries from prior versions (existing flagged + behavior, untouched). +- Changing the runtime parameter-injection / terraform path. + +## Decisions + +### 1. Pin "latest" at install time, not run time +Install resolves an empty version to a concrete release tag via the GitHub +Releases API and stores that concrete tag in the row. Runs are therefore +reproducible and the version-keyed cache path stays valid. +**Alternative considered:** resolve "latest" lazily at each run. Rejected — the +pack would silently drift between runs and the cache key for "latest" is +ill-defined. + +### 2. Derive name (and version) from the pack manifest for all types +After the binary is on disk, install runs its `manifest` command and uses +`pack.name` / `pack.version` as the persisted identity. The `name` field is +removed from the dialog and ignored in `InstallPackRequest`. +**Alternative considered:** default `name` to the repo name. Rejected — the +manifest is the authoritative identity and unifies all three types behind one +"resolve binary → manifest → upsert" path. + +### 3. Resolve remote artifacts via the GitHub Releases API +`GET /releases/latest` (empty version) or `/releases/tags/v{version}` returns the +tag plus the asset list. The platform archive is the asset whose name matches +`*_{GOOS}_{GOARCH}.tar.gz`; the in-tarball binary name is derived from that +asset's filename prefix (and, if the archive contains a single executable, that +file is taken). This removes the artifact-prefix coupling entirely. +**Alternative considered:** keep building the URL by hand but add a separate +`artifact_name` field. Rejected — still makes the operator know the goreleaser +project name; the API already has the answer. + +### 4. Install becomes a network/manifest operation with a temp-stage step +Because the name is unknown until the manifest runs, remote/upload download or +write the binary to a temp location, run the manifest, then relocate to the +canonical path (`packs/{name}/{tag}/{binary}` for remote, `packs/{name}/upload/` +for upload). Local references the binary in place and only runs its manifest. + +### 5. Upsert-by-name semantics unchanged +The manifest-derived `name` remains the unique DB key. Two different sources that +report the same `pack.name` overwrite each other, exactly as today's name-keyed +upsert does. Not special-cased. + +## Risks / Trade-offs + +- **GitHub API rate limit (60/hr per IP, unauthenticated)** → Acceptable for + occasional installs; surface a clear error if a `latest` lookup is rate-limited + so the operator can retry or pin an explicit version. +- **Local install now requires a runnable binary at install time** (drops the + "succeeds with a non-existent path" behavior) → This is intentional fail-loud + behavior; documented as a breaking change in the spec delta. +- **Install can now fail for many new reasons** → Each failure mode returns a + specific, actionable error (repo not found, no asset for `{os}/{arch}` in + `{tag}`, >1 matching asset listing candidates, checksum mismatch, manifest + error). No partial DB row is created on failure. +- **Ambiguous in-tarball binary name** if a release deviates from the goreleaser + convention → Derive from the asset prefix first; fall back to the single + executable in the archive; error if neither resolves. diff --git a/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/proposal.md b/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/proposal.md new file mode 100644 index 0000000..4f5b7bb --- /dev/null +++ b/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/proposal.md @@ -0,0 +1,72 @@ +## Why + +Installing a pack is unnecessarily hard. The single `name` field is overloaded +as three different things — the GitHub release artifact filename prefix +(`{name}_{version}_{os}_{arch}.tar.gz`), the binary name inside the tarball, and +the simrun DB/display identifier. Because a pack repo's goreleaser project/binary +name often differs from its repo name, the operator has to reverse-engineer the +exact artifact name to make the source and name line up. On top of that, `version` +is required and must match a release exactly, with no way to ask for "latest". + +## What Changes + +- **BREAKING**: Remove `name` from the install flow. `POST /api/packs/install` + ignores any `name` in the request; the install dialog drops the name field + entirely. The pack's identity comes from its own manifest instead. +- Install becomes a real operation (today it only writes metadata). For every + pack type, install now makes the binary available, runs the pack's `manifest` + command, and persists the row using the manifest's `pack.name` and + `pack.version`. Failures (bad repo, missing artifact, checksum mismatch, + manifest error, `min_simrun_version` mismatch) surface at install time, not at + first run. +- **Remote**: `version` is now optional. Resolution goes through the GitHub + Releases API — `/releases/latest` when version is empty, otherwise + `/releases/tags/v{version}`. The platform artifact is selected from the + release's asset list by matching `*_{os}_{arch}.tar.gz` (so the artifact prefix + no longer has to equal anything the operator types), checksum-verified against + `checksums.txt`, extracted, and the resolved concrete tag is pinned into the + stored row. +- **Local**: install now verifies the path exists and runs its manifest at + install time (dropping today's "install can succeed with a non-existent path" + behavior). The binary is still referenced in place, not copied. +- **Upload**: the uploaded binary's manifest is run to derive name + version; the + binary is relocated under `/packs/{name}/upload/`. +- Remote support stays public-GitHub-only and anonymous (no token). GitHub + Releases API `latest` lookups are rate-limited to 60/hr per IP unauthenticated, + which is acceptable for occasional installs. + +## Capabilities + +### New Capabilities + + +### Modified Capabilities +- `packs`: Rework the install lifecycle. Modify "Remote Pack Install" + (API-based resolution, optional version → latest, manifest-derived name, + install-time download + verify), "Local Pack Install" (verify + run manifest at + install; remove the non-existent-path note), "Upload Pack Install" + (manifest-derived name + version), and "Install Is Idempotent By Name" (name is + now manifest-derived). Add scenarios for latest resolution, no-asset-for-platform, + install-fails-on-bad-repo, and manifest-derived name. "Source Format Validation" + is unchanged. + +## Impact + +- **Backend**: + - `internal/packs/resolver/resolver.go` — add GitHub Releases API resolution and + asset matching; drop the hand-built artifact filename; `version` no longer + required; derive the in-tarball binary name from the chosen asset. + - `internal/web/packs_handler.go` — `HandleInstallPack` and the upload handler + move to an eager resolve → run-manifest → derive-name → upsert pipeline. + - `internal/web/types.go` — `InstallPackRequest.Name` becomes optional/ignored. +- **Frontend**: `web/frontend/src/routes/packs/+page.svelte` — remove the name + field; type-dependent inputs (remote = source + optional version, local = path, + upload = file). +- **API**: `POST /api/packs/install` no longer requires `name`; remote `version` + optional; install can now fail with download/manifest errors. +- **Runtime**: unchanged — `Factory.CreateRunner` → `Resolver.Resolve` stays + cache-keyed by `name`+`version` and hits the cache populated at install. +- **Source of truth**: `pack/protocol.go` `PackInfo.Name`/`Version` already exist. +- **Specs**: `openspec/specs/packs/spec.md`. +- **Dependencies**: none added (uses the existing HTTP client against the public + GitHub API). diff --git a/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/specs/packs/spec.md b/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/specs/packs/spec.md new file mode 100644 index 0000000..c7c9e56 --- /dev/null +++ b/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/specs/packs/spec.md @@ -0,0 +1,110 @@ +## MODIFIED Requirements + +### Requirement: Remote Pack Install +The system SHALL fetch `remote` packs through the GitHub Releases API. When +the request omits a version, the system SHALL resolve the release via +`GET /repos///releases/latest`; when a version is provided, via +`GET /repos///releases/tags/v`. The concrete resolved tag +SHALL be persisted as the pack's `version`. From the release's assets the system +SHALL select the archive whose name matches `*__.tar.gz`, fetch the +release's `checksums.txt` asset, verify the archive's SHA-256 against it, extract +the pack binary, and cache it at `/packs///` — where +`` is derived from the manifest (see "Install Derives Identity From +Manifest"). The in-tarball binary name SHALL be derived from the selected asset's +filename prefix; if the archive contains a single executable, that file SHALL be +used. The system SHALL NOT require the operator to supply the artifact filename +or the binary name. + +#### Scenario: Successful download of a pinned version +- **WHEN** a client installs a `remote` pack from `github.com/org/repo` with version `1.2.3` +- **THEN** the release at tag `v1.2.3` is resolved, the matching `*__.tar.gz` asset is checksum-verified and extracted, the binary is cached under `/packs//1.2.3/`, and the row's `version` is `1.2.3` + +#### Scenario: Latest resolution when version omitted +- **WHEN** a client installs a `remote` pack from `github.com/org/repo` with no version +- **THEN** the system resolves the latest release via the Releases API, persists the row's `version` as that release's concrete tag, and downloads that release's platform asset + +#### Scenario: Checksum mismatch +- **WHEN** the downloaded archive's SHA-256 does not match the entry in `checksums.txt` +- **THEN** the install fails with an error and no DB record is created + +#### Scenario: No asset for the current platform +- **WHEN** the resolved release contains no asset matching `*__.tar.gz` +- **THEN** the install fails with an error naming the `/` and the resolved tag, and no DB record is created + +#### Scenario: Multiple matching assets +- **WHEN** the resolved release contains more than one asset matching `*__.tar.gz` +- **THEN** the install fails with an error listing the candidate asset names, and no DB record is created + +#### Scenario: Repository or release not found +- **WHEN** the GitHub Releases API returns not-found for the repo or the requested tag +- **THEN** the install fails with an error and no DB record is created + +#### Scenario: Source format +- **WHEN** a client provides `source: "https://github.com/org/repo"` or `"github.com/org/repo"` +- **THEN** both forms are accepted (HTTP/HTTPS prefixes are stripped) + +### Requirement: Local Pack Install +The system SHALL accept `local` packs whose `source` is an absolute filesystem +path to an existing binary. The install endpoint SHALL NOT copy the binary; it +SHALL reference the path as-is at run time. At install time the system SHALL +verify the path exists and SHALL run the binary's `manifest` command to derive +the pack's identity (see "Install Derives Identity From Manifest"). An install +SHALL fail if the path does not exist or the manifest command fails. + +#### Scenario: Path stored verbatim +- **WHEN** a client installs a local pack with `source: "/opt/packs/my-pack"` that exists and returns a valid manifest +- **THEN** the persisted row has `source = "/opt/packs/my-pack"` and the name/version come from the manifest + +#### Scenario: Non-existent path rejected at install +- **WHEN** a client installs a local pack whose `source` path does not exist +- **THEN** the install fails with an error and no DB record is created + +### Requirement: Upload Pack Install +The system SHALL accept binary uploads via `POST /api/packs/upload` +(multipart form). The system SHALL write the binary to a temporary location, run +its `manifest` command to derive the pack's identity (see "Install Derives +Identity From Manifest"), relocate the binary to +`/packs//upload/`, and persist the `source` as that path. +Runtime SHALL treat `upload` packs identically to `local` packs. An install SHALL +fail if the manifest command fails, and no DB record SHALL be created. + +#### Scenario: Successful upload +- **WHEN** a client uploads a binary that returns a valid manifest as multipart form data +- **THEN** the binary is written to `/packs//upload/`, a `packs` row is created with `type = "upload"`, and the name/version come from the manifest + +#### Scenario: Manifest failure rejects upload +- **WHEN** an uploaded binary's `manifest` command fails +- **THEN** the install fails with an error and no DB record is created + +### Requirement: Install Is Idempotent By Name +The system SHALL upsert by the manifest-derived `name` on `POST /api/packs/install` +and `POST /api/packs/upload`. Subsequent installs that resolve to the same +manifest `name` replace the existing row's type, source, and version. Cached +binaries from previous installs SHALL NOT be cleaned up by the upsert. +**Note:** flagged — disk usage grows when versions change repeatedly. + +#### Scenario: Reinstall replaces row +- **WHEN** a pack whose manifest reports name `p1` is installed at v1.0.0 then a newer release reporting the same name `p1` is installed at v2.0.0 +- **THEN** the row's `version` is `2.0.0`, but the v1.0.0 binary remains under `/packs/p1/1.0.0/` + +## ADDED Requirements + +### Requirement: Install Derives Identity From Manifest +The system SHALL derive a pack's `name` and `version` from the binary's +`manifest` command (`pack.name` and `pack.version`) at install time, for all pack +types. The system SHALL ignore any `name` supplied in the install request. The +install SHALL run the manifest only after the binary is available (downloaded for +`remote`, present on disk for `local`, written to a temp location for `upload`), +and SHALL fail without creating a DB record if the manifest command fails. + +#### Scenario: Name comes from manifest, not request +- **WHEN** a client posts an install request whose body contains `name: "operator-typed"` and the resolved binary's manifest reports `pack.name: "real-pack"` +- **THEN** the persisted row's `name` is `real-pack` and the request's `name` is ignored + +#### Scenario: Version pinned from manifest or resolved tag +- **WHEN** an install completes successfully +- **THEN** the persisted `version` reflects the resolved release tag (remote) or the manifest's `pack.version` (local/upload), never an operator-typed value beyond an optional remote version pin + +#### Scenario: Manifest failure aborts install +- **WHEN** the `manifest` command fails for the resolved binary +- **THEN** the install fails with an error and no DB record is created diff --git a/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/tasks.md b/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/tasks.md new file mode 100644 index 0000000..e03a5c1 --- /dev/null +++ b/openspec/changes/archive/2026-07-01-manifest-derived-pack-install/tasks.md @@ -0,0 +1,30 @@ +## 1. Resolver: GitHub Releases API + manifest-driven resolution + +- [x] 1.1 Add a GitHub Releases API client call in `internal/packs/resolver/resolver.go`: `GET /repos///releases/latest` when version is empty, else `/repos///releases/tags/v`; return the concrete tag and the asset list (name + browser_download_url). Map not-found and rate-limit responses to clear errors. +- [x] 1.2 Replace `buildDownloadURLs` (hand-built `{name}_{version}_{os}_{arch}.tar.gz`) with asset selection from the release: pick the asset matching `*__.tar.gz`; error on 0 matches (name os/arch + tag) and on >1 matches (list candidates). Locate the `checksums.txt` asset from the same list. +- [x] 1.3 Update `validatePackConfig`: remove the "version required" check; keep `source` required for remote. +- [x] 1.4 Update `extractTarGz` / extraction: derive the expected in-tarball binary name from the selected asset's filename prefix; fall back to the single executable in the archive; error if neither resolves. Cache at `/packs///` using a temp-stage-then-relocate flow so the path can use the manifest-derived name. +- [x] 1.5 Keep checksum verification against `checksums.txt` for the selected asset. + +## 2. Install pipeline: resolve → manifest → derive identity → upsert + +- [x] 2.1 Add a shared helper (e.g. in `internal/packs/` or the handler) that, given an available binary path, runs the pack `manifest` command and returns `pack.name` / `pack.version` (`pack/protocol.go` `PackInfo`). Surface manifest failures as install errors. (Reused existing `resolver.GetManifest`.) +- [x] 2.2 Rework `HandleInstallPack` in `internal/web/packs_handler.go` into an eager pipeline: for `remote` download+verify+extract to temp; for `local` verify the path exists; then run the manifest, derive name+version, relocate (remote), and upsert. No DB row on any failure. +- [x] 2.3 Rework the upload handler: write the uploaded binary to temp, run the manifest, relocate to `/packs//upload/`, upsert with manifest name+version; fail without a row on manifest error. +- [x] 2.4 Make `InstallPackRequest.Name` in `internal/web/types.go` optional/ignored; ensure the type validation (local/remote/upload) still runs. +- [x] 2.5 Confirm `Factory.CreateRunner` → `Resolver.Resolve` still finds the install-time cached binary by `name`+`version` (cache hit, no re-download); adjust only if the relocate path changed the cache key. (No change: relocate path matches the `cachedBinary` scan dir.) + +## 3. Frontend install dialog + +- [x] 3.1 In `web/frontend/src/routes/packs/+page.svelte` remove the name input and make inputs type-dependent: remote = source + optional version, local = absolute path, upload = file picker. (Per decision: dialog offers remote + upload only; no local option.) +- [x] 3.2 Update the `installPack` client call / payload to stop sending `name`; surface install-time errors (bad repo, no asset, checksum, manifest) in the existing error UI. + +## 4. Tests + +- [x] 4.1 Resolver tests: latest resolution (empty version → concrete tag), tag resolution, asset matching (0 / 1 / >1 matches), checksum mismatch, repo/tag not found, binary-name derivation from asset prefix and single-executable fallback. +- [x] 4.2 Install handler tests: name ignored from request and taken from manifest; version pinned from resolved tag (remote) and manifest (local/upload); no DB row on download/manifest failure; local non-existent path rejected. +- [x] 4.3 Run `go test ./...` and `mise run lint`; build with `mise run build`. + +## 5. Spec sync + +- [x] 5.1 After implementation, apply the spec delta to `openspec/specs/packs/spec.md` (handled at archive time) and confirm scenarios match the shipped behavior. diff --git a/openspec/changes/archive/2026-07-06-org-default-tags/.openspec.yaml b/openspec/changes/archive/2026-07-06-org-default-tags/.openspec.yaml new file mode 100644 index 0000000..8e26fbe --- /dev/null +++ b/openspec/changes/archive/2026-07-06-org-default-tags/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-02 diff --git a/openspec/changes/archive/2026-07-06-org-default-tags/design.md b/openspec/changes/archive/2026-07-06-org-default-tags/design.md new file mode 100644 index 0000000..8c9d318 --- /dev/null +++ b/openspec/changes/archive/2026-07-06-org-default-tags/design.md @@ -0,0 +1,53 @@ +# Design: Org-wide Default Tags + +## Context + +`default_tags` is a built-in pack param: the pack SDK guarantees a `variable "default_tags"` block exists in every sim's Terraform and includes it in every pack's `params_schema`, so injecting a value for it is safe for all packs. Today the only persistence is `packs.parameters` (JSONB); `HandleDeletePack` → `packStore.Delete` hard-deletes the row, so tags are lost on remove/reinstall. `app_config` is a generic KV table (`key` TEXT PK, `value` JSONB) with a typed `AppConfig` view (`internal/config/appconfig.go`, mapped in `internal/db/config.go`), and `PUT /api/config` already does per-key validation for retention days. + +Alternatives rejected during exploration: named parameter presets with copy-on-apply (doesn't survive reinstall without manual re-apply; drift between preset and stamped copies) and live preset references from packs (merge/FK complexity disproportionate to the problem). + +## Goals / Non-Goals + +**Goals:** +- Org default tags entered once, applied to every pack's detonations, surviving pack remove/reinstall. +- Per-key override precedence: TF variable default < org default tag < pack-level tag < per-sim scenario param. +- Users can see the effective tag set in the pack parameters dialog. + +**Non-Goals:** +- Settings page redesign / proper tags editor UI (separate change: `settings-page-redesign`). +- Tombstones (a pack cannot delete an org tag, only override its value). +- Org-level defaults for any other parameter (`aws_region`, `gcp_project`, custom params). +- Multiple named tag sets ("presets"). + +## Decisions + +### D1: Store in `app_config`, not a new table +One org-wide value, no versioning, no relations — the existing KV table with a typed `AppConfig.DefaultTags map[string]string` field is the established pattern (parallelism, retention, logging flags all live there). A migration backfills `default_tags = {}` following the migration-008 pattern so the key shows up in `GET /api/config`. + +### D2: Merge in `loadPacksFromDB`, not the detonator +`ScenarioService.loadPacksFromDB` is the single place DB pack rows become `config.PackConfig`, and it feeds both `Run` and `Lint`. Merging there means every consumer (parser manifest calls, detonator TF_VAR promotion) sees effective parameters, and `internal/detonators/simrun_detonator.go` stays untouched — its existing pack-level < per-sim precedence keeps working on the already-merged map. Alternative (merge inside the detonator) rejected: the detonator has no access to `AppConfig` and would need new plumbing. + +Merge rule: `effective = org map, overlaid per-key by pack's parameters["default_tags"]` (when present and a string map). If the pack has no `default_tags` key and the org map is non-empty, the merged org map is set as `parameters["default_tags"]`. Empty org map is a no-op — `parameters` passes through unchanged. A malformed pack-level value (not a string map) is left as-is and org tags are not merged into it, preserving current behavior. + +### D3: Validate `default_tags` in `HandleUpdateConfig` +Add a per-key branch (same shape as the retention-days validation): the value must decode as `map[string]string`. Prevents the generic KV editor from storing a string/array that would later break the merge. Other keys keep permissive behavior. + +### D4: Pack dialog shows inheritance read-only, fetched from config API +`PackParametersDialog.svelte` additionally calls the existing `getConfig()` and renders org tags as muted, non-editable rows in the `default_tags` section, labeled as inherited from Settings; a pack-level entry with the same key visually marks the inherited row as overridden. Display only — saving still writes only the pack's own entries to `packs.parameters`. This keeps the copy-drift problem out: merged values are never persisted per-pack. + +## Risks / Trade-offs + +- [Users with existing per-pack tags see no behavior change until they set org tags] → intended; org map defaults to `{}` (no-op), rollout is opt-in. +- [Per-sim `default_tags` param replaces the whole map, not per-key] → existing behavior for pack-level vs per-sim, unchanged; documented in the spec scenario. +- [Until `settings-page-redesign` lands, org tags are edited as raw JSON on the Configuration page] → acceptable interim; D3 validation rejects malformed input at save time. +- [Lint/manifest calls now see merged parameters] → intended (consistent view), but worth verifying no manifest-time validation rejects org-injected tags for packs whose schema marks `default_tags` differently; `default_tags` is a built-in present in every schema, so this is theoretical. + +## Migration Plan + +1. Deploy migration backfilling `default_tags = '{}'::jsonb` (idempotent `ON CONFLICT DO NOTHING`). +2. Deploy backend + frontend together (single binary). +3. Rollback: revert deploy; the extra `app_config` row is inert for older code (unknown keys are ignored by `parseAppConfig`). + +## Open Questions + +None — precedence, storage, and merge point were settled during exploration. diff --git a/openspec/changes/archive/2026-07-06-org-default-tags/proposal.md b/openspec/changes/archive/2026-07-06-org-default-tags/proposal.md new file mode 100644 index 0000000..de6c0f0 --- /dev/null +++ b/openspec/changes/archive/2026-07-06-org-default-tags/proposal.md @@ -0,0 +1,31 @@ +# Org-wide Default Tags + +## Why + +`default_tags` (owner, cost-center, `simulated:true`, …) are org-wide standards, but today they only live in per-pack `packs.parameters`. Users retype them for every pack, and they are lost whenever a pack is removed and reinstalled because pack deletion hard-deletes the row. An app-level setting fixes both: enter once, survives pack lifecycle. + +## What Changes + +- Add `default_tags` (string→string map, default `{}`) to `AppConfig`, stored under the `default_tags` key in the existing `app_config` KV table and carried by the existing `GET/PUT /api/config` endpoints. `PUT` validates the value is a string→string object (same per-key validation pattern as retention days). +- Migration backfills the `default_tags` key with `{}` so it appears in `GET /api/config` output. +- When the scenario service builds pack configs from the DB (`loadPacksFromDB`), org default tags are merged per-key beneath each pack's own `default_tags`: org value < pack-level value < per-sim scenario param. Packs override individual tags; they cannot delete an org tag. +- Pack parameters dialog shows org default tags as read-only inherited rows (sourced from `GET /api/config`) so users see the effective tag set; a pack-level entry with the same key visibly overrides the inherited one. The dialog never writes merged values into `packs.parameters`. + +Editing org default tags uses the existing Configuration page's generic key/value editing (JSON object in the text field) until the settings page redesign (separate change `settings-page-redesign`) ships a proper key/value editor. + +## Capabilities + +### New Capabilities + +- `app-settings`: App-level admin settings — the org-wide `default_tags` setting: storage shape, config API validation, and inherited-tags visibility in the pack parameters dialog. + +### Modified Capabilities + +- `pack-execution`: Parameter injection gains a new bottom layer — org-wide default tags are merged per-key beneath pack-level `default_tags` before promotion to `TF_VAR_default_tags`. + +## Impact + +- **Backend**: `internal/config/appconfig.go` (`DefaultTags` field + default), `internal/db/config.go` (KV mapping), new migration (backfill `default_tags` key), `internal/web/handlers.go` (`HandleUpdateConfig` validation branch), `internal/web/scenarios.go` (`loadPacksFromDB` merge). +- **Frontend**: `PackParametersDialog.svelte` (inherited rows). No settings page changes in this proposal. +- **APIs**: no new endpoints; existing config GET/PUT carries the new key. +- **No breaking changes**: existing pack-level `default_tags` keep working and take precedence over org defaults; empty org map is a no-op. diff --git a/openspec/changes/archive/2026-07-06-org-default-tags/specs/app-settings/spec.md b/openspec/changes/archive/2026-07-06-org-default-tags/specs/app-settings/spec.md new file mode 100644 index 0000000..6f9146e --- /dev/null +++ b/openspec/changes/archive/2026-07-06-org-default-tags/specs/app-settings/spec.md @@ -0,0 +1,57 @@ +# App Settings Delta + +## ADDED Requirements + +### Requirement: Org-wide Default Tags Setting +The system SHALL store an org-wide default tags value as a string→string +map under the `default_tags` key in the `app_config` table, defaulting to +an empty map, exposed through the typed `AppConfig` as `DefaultTags`. The +key SHALL be backfilled by migration so it appears in `GET /api/config` +responses. + +#### Scenario: Default value present after migration +- **WHEN** the server starts against a database without a `default_tags` row +- **THEN** migrations create the row with value `{}` and `GET /api/config` includes `default_tags` + +#### Scenario: Survives pack lifecycle +- **WHEN** org default tags are set, and a pack is removed and reinstalled +- **THEN** the org default tags are unchanged and still apply to the reinstalled pack's detonations + +### Requirement: Default Tags Update Validation +`PUT /api/config` with key `default_tags` SHALL reject any value that is +not a JSON object whose values are all strings, returning HTTP 400 and +not persisting the value. Valid string→string objects (including the +empty object) SHALL be stored. + +#### Scenario: Non-object rejected +- **WHEN** a client sends `PUT /api/config` with key `default_tags` and value `"owner=secops"` +- **THEN** the server responds 400 and the stored value is unchanged + +#### Scenario: Non-string tag value rejected +- **WHEN** a client sends `PUT /api/config` with key `default_tags` and value `{"owner": 123}` +- **THEN** the server responds 400 and the stored value is unchanged + +#### Scenario: Valid map stored +- **WHEN** a client sends `PUT /api/config` with key `default_tags` and value `{"owner": "secops", "simulated": "true"}` +- **THEN** the server responds 204 and `GET /api/config` returns the new map + +### Requirement: Inherited Tags Visible in Pack Parameters Dialog +The pack parameters dialog SHALL display org-wide default tags as +read-only inherited entries within the `default_tags` field, visually +distinct from the pack's own entries and attributed to app settings. +When a pack-level entry uses the same key as an inherited entry, the +dialog SHALL indicate that the inherited value is overridden. Saving the +dialog SHALL persist only the pack's own entries to `packs.parameters`, +never the merged result. + +#### Scenario: Inherited tags shown read-only +- **WHEN** org default tags are `{"owner": "secops"}` and a user opens a pack's parameters dialog +- **THEN** `owner: secops` is shown as a non-editable inherited entry in the `default_tags` section + +#### Scenario: Override indicated +- **WHEN** org default tags contain `owner: secops` and the pack's own `default_tags` contain `owner: red-team` +- **THEN** the dialog indicates the inherited `owner` value is overridden by the pack-level entry + +#### Scenario: Merged values never persisted per-pack +- **WHEN** org default tags are `{"owner": "secops"}`, the pack's own `default_tags` are `{"team": "red"}`, and the user saves the dialog without edits +- **THEN** `packs.parameters.default_tags` contains only `{"team": "red"}` diff --git a/openspec/changes/archive/2026-07-06-org-default-tags/specs/pack-execution/spec.md b/openspec/changes/archive/2026-07-06-org-default-tags/specs/pack-execution/spec.md new file mode 100644 index 0000000..6e751fb --- /dev/null +++ b/openspec/changes/archive/2026-07-06-org-default-tags/specs/pack-execution/spec.md @@ -0,0 +1,68 @@ +# Pack Execution Delta + +## MODIFIED Requirements + +### Requirement: Parameter Injection Two Ways +The system SHALL pass pack parameters (from DB `packs.parameters`) into +both the `ManifestInput.Parameters` field on the manifest call and as +`TF_VAR_=` environment variables for terraform invocations. +Per-scenario `params:` from the YAML SHALL be merged into +`DetonateInput.Params` and ALSO promoted to `TF_VAR_*` env vars. When +both scopes set the same key, the per-scenario value SHALL take +precedence over the pack-level value; both layers SHALL be applied on +top of any `default = ...` declared on the matching Terraform +`variable` block. Pack-level values SHALL be passed for every key in +`packs.parameters`, regardless of whether the value was declared in +the pack's `params_schema` (so previously-stored unknown keys continue +to flow until cleaned up). + +For the `default_tags` key, org-wide default tags from app settings +SHALL be merged per-key beneath the pack-level value when pack +configurations are built from the database: an org tag applies unless +the pack's own `default_tags` sets the same key, and packs cannot +delete an org tag (no tombstones). The full precedence for a tag key is: +Terraform `variable` default < org-wide default tag < pack-level +`default_tags` entry < per-scenario `params`. An empty org map SHALL +leave pack parameters unchanged, and a pack-level `default_tags` value +that is not a string→string map SHALL pass through unmodified with no +org merge applied. + +#### Scenario: Parameter as TF_VAR +- **WHEN** the pack DB record has `parameters: {"region":"us-east-1"}` +- **THEN** terraform inside the detonation runs with `TF_VAR_region=us-east-1` + +#### Scenario: Per-scenario override of pack-level value +- **WHEN** the pack record has `parameters: {"aws_region": "us-east-1"}` + and a scenario YAML sets `params: { aws_region: "us-west-2" }` for one + simulation +- **THEN** that simulation's `terraform apply` runs with + `TF_VAR_aws_region=us-west-2`, while sims without a scenario-level + override still see `us-east-1` + +#### Scenario: TF variable default used when neither scope provides value +- **WHEN** a sim's TF declares `variable "resource_prefix" { default = "simrun" }` + and neither the pack record nor the scenario sets `resource_prefix` +- **THEN** terraform uses the declared default of `"simrun"` + +#### Scenario: Pack-level unknown keys still flow +- **WHEN** the pack's stored parameters include a key not present in + the pack's `params_schema` (e.g., a legacy key) +- **THEN** the key is still exported as a `TF_VAR_*` env var to the + detonation's terraform process + +#### Scenario: Org default tag applies to pack without own value +- **WHEN** org default tags are `{"owner": "secops"}` and the pack's + stored parameters have no `default_tags` key +- **THEN** the detonation's terraform runs with + `TF_VAR_default_tags={"owner":"secops"}` + +#### Scenario: Pack-level tag overrides org tag per-key +- **WHEN** org default tags are `{"owner": "secops", "env": "sim"}` and + the pack's `default_tags` are `{"owner": "red-team"}` +- **THEN** the effective map is `{"owner": "red-team", "env": "sim"}` + +#### Scenario: Empty org map is a no-op +- **WHEN** org default tags are `{}` and the pack's `default_tags` are + `{"team": "red"}` +- **THEN** the effective map is `{"team": "red"}`, identical to behavior + before org-wide default tags existed diff --git a/openspec/changes/archive/2026-07-06-org-default-tags/tasks.md b/openspec/changes/archive/2026-07-06-org-default-tags/tasks.md new file mode 100644 index 0000000..5f82354 --- /dev/null +++ b/openspec/changes/archive/2026-07-06-org-default-tags/tasks.md @@ -0,0 +1,26 @@ +# Tasks: Org-wide Default Tags + +## 1. Backend storage and API + +- [x] 1.1 Add `DefaultTags map[string]string` to `AppConfig` in `internal/config/appconfig.go` with `{}` default in `DefaultAppConfig` +- [x] 1.2 Map the `default_tags` KV key in `internal/db/config.go` (`parseAppConfig` / `appConfigKVs`) +- [x] 1.3 Add migration backfilling `default_tags = '{}'::jsonb` into `app_config` (idempotent, follows migration 008 pattern) +- [x] 1.4 Add `default_tags` validation branch in `HandleUpdateConfig` (`internal/web/handlers.go`): value must decode as `map[string]string`, else 400 +- [x] 1.5 Tests: config store round-trip for `DefaultTags`; handler tests for valid map, non-object, and non-string-value payloads (extend `api_config_test.go`) + +## 2. Merge into pack parameters + +- [x] 2.1 In `ScenarioService.loadPacksFromDB` (`internal/web/scenarios.go`), load `AppConfig` and merge org default tags per-key beneath each pack's `parameters["default_tags"]` per design D2 (empty org map = pass-through; malformed pack-level value = pass-through, no merge) +- [x] 2.2 Tests encoding the precedence contract: org tag applies when pack has none; pack-level key wins per-key; empty org map leaves parameters byte-identical; malformed pack-level `default_tags` untouched +- [x] 2.3 Verify end-to-end that the merged map reaches `TF_VAR_default_tags` via the existing detonator promotion (existing detonator tests still pass; add one if the merged path is not covered) + +## 3. Pack parameters dialog inheritance + +- [x] 3.1 In `PackParametersDialog.svelte`, fetch org config via existing `getConfig()` alongside parameters/manifest and extract `default_tags` +- [x] 3.2 Render inherited tags as read-only muted rows in the `default_tags` section, attributed to Settings; mark inherited rows overridden when a pack-level entry uses the same key +- [x] 3.3 Confirm save still submits only pack-level entries (no merged values in the PUT body) + +## 4. Verification + +- [x] 4.1 `go test ./...` and `mise run lint` pass +- [x] 4.2 Manual flow: set org tags via `PUT /api/config`, open pack params dialog (inherited rows visible), remove/reinstall a pack, run a scenario, confirm `TF_VAR_default_tags` includes org tags diff --git a/openspec/changes/settings-page-redesign/.openspec.yaml b/openspec/changes/settings-page-redesign/.openspec.yaml new file mode 100644 index 0000000..8e26fbe --- /dev/null +++ b/openspec/changes/settings-page-redesign/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-02 diff --git a/openspec/changes/settings-page-redesign/design.md b/openspec/changes/settings-page-redesign/design.md new file mode 100644 index 0000000..51fe77e --- /dev/null +++ b/openspec/changes/settings-page-redesign/design.md @@ -0,0 +1,49 @@ +# Design: Settings Page Redesign + +## Context + +`/config` (`src/routes/config/+page.svelte`, 156 lines) iterates `Object.entries(config)` and renders each key as `mono label + text Input + Save`. `AppConfig` has 8 known keys (parallelism, terraform_version, pack_logs_enabled, ssh_logging_enabled, run_log_retention_enabled/days, run_retention_enabled/days) plus `default_tags` after the `org-default-tags` change. The frontend follows shadcn-svelte (nova baseline) with restraint-first design taste: quiet, data-grounded, no decorative flourishes. `SchemaForm.svelte` already contains a MapEntry key/value editor used for `default_tags` in the pack parameters dialog. + +## Goals / Non-Goals + +**Goals:** +- Typed, grouped, human-labeled settings in four tabs: General, Default tags, Retention, About. +- A proper key/value editor for org default tags, shared with the pack parameters dialog. +- Remove dead generic machinery (KV loop, sensitive masking). + +**Non-Goals:** +- Any backend or API change. +- Settings search, audit history, or per-user preferences. +- A settings sidebar/sub-navigation (four tabs suffice at this scale). + +## Decisions + +### D1: Tabs over a single sectioned page +User decision. Four tabs, each substantive: **General** absorbs execution + logging so the landing tab isn't hollow; **Default tags** gets its own surface (the editor plus precedence explanation); **Retention** stands alone because it deletes history — different stakes than tuning execution; **About** separates read-only from editable. Uses installed `Tabs`. + +### D2: Hardcoded settings metadata over generic rendering +The frontend declares each setting's label, description, control type, and tab. Rationale: human labels/descriptions require per-key knowledge anyway; the generic loop's only advantage (rendering unknown keys) is not worth booleans-as-text. Trade-off: a future config key requires a frontend change — accepted, it effectively already does. The sensitive-key masking heuristic is deleted (secrets live in secret groups; `app_config` has none). + +### D3: Save model — switches commit on toggle, one Save per tab otherwise +Switches are self-describing transactions (existing `PUT /api/config` is per-key, so a toggle is one call). Text/number fields batch into a single Save per tab issuing one PUT per dirty key, replacing eight per-row buttons. Retention day inputs are disabled while their enable switch is off. Errors surface via the existing `Alert` pattern. + +### D4: Shared string-map editor extracted from `SchemaForm.svelte` +The MapEntry editor (entries state, `objectToEntries`/`entriesToObject`, blank key/value validation) moves to a shared component (e.g. `$lib/components/KeyValueEditor.svelte`); `SchemaForm` delegates to it for `object` + `additionalProperties.type: "string"` properties, and the Default tags tab uses it directly. One editor, one behavior, both surfaces. It also accepts read-only inherited rows so the pack dialog's inheritance display (from `org-default-tags`) renders through the same component. + +### D5: Composition from existing components; add `field` +`Tabs`, `Card`, `Switch`, `Input`, `InputGroup` (days suffix), `Alert`, `Skeleton` are installed. Add shadcn-svelte `field` and build forms with `Field.FieldGroup`/`Field.Field` per the project's forms convention. No custom-styled divs; restraint-first — no new visual vocabulary, the page should read as the same app. + +## Risks / Trade-offs + +- [Unknown/legacy `app_config` keys become invisible in the UI] → accepted; still reachable via API. Known keys are exactly the `AppConfig` struct. +- [Frontend and backend key lists can drift] → the typed `AppConfig` struct is the source of truth; task includes aligning the frontend `AppConfig` TS type. +- [`SchemaForm` regression while extracting the map editor] → pack parameters dialog is exercised manually as part of verification; extraction is move-not-rewrite. +- [Tab state lost on reload] → acceptable at this scale; General is the default tab. + +## Migration Plan + +Frontend-only rewrite behind the same route; no data or API migration. Rollback = revert the frontend commit. Implement after `org-default-tags` so the Default tags tab edits a validated, existing key. + +## Open Questions + +None. diff --git a/openspec/changes/settings-page-redesign/proposal.md b/openspec/changes/settings-page-redesign/proposal.md new file mode 100644 index 0000000..21664c1 --- /dev/null +++ b/openspec/changes/settings-page-redesign/proposal.md @@ -0,0 +1,35 @@ +# Settings Page Redesign + +## Why + +The Configuration page (`/config`) renders every `app_config` key through a generic loop: raw system keys in monospace as labels, every value — including booleans and numbers — as a text input, one Save button per row, plus a sensitive-key masking heuristic that is dead weight now that credentials live in secret groups. It cannot reasonably host the org-wide `default_tags` map (change `org-default-tags`), which today would be edited as raw JSON in a text box. + +## What Changes + +- Replace the generic KV loop with a tabbed Settings page using typed controls and human labels/descriptions for the known `AppConfig` keys: + - **General**: parallelism (number), terraform version (text), pack logs (switch), SSH session logging (switch) + - **Default tags**: key/value editor for the org-wide `default_tags` setting, with a description of precedence (packs override individual tags) + - **Retention**: run log retention and run retention — each an enable switch paired with a days input, days disabled while the switch is off + - **About**: read-only version info (version, commit, build date, Go version) +- Switches save on toggle; tabs with text/number inputs get one Save action per tab. +- Remove the generic key/value rendering and the sensitive-key masking heuristic; settings metadata (label, description, control type, grouping) is hardcoded in the frontend. +- Extract the string-map (key/value) editor from `SchemaForm.svelte` into a shared component used by both the pack parameters dialog and the Default tags tab. +- Add the shadcn-svelte `field` component; forms use `Field.FieldGroup`/`Field.Field`. + +Depends on `org-default-tags` for the `default_tags` key and its server-side validation; implement that change first. + +## Capabilities + +### New Capabilities + +- `settings-page`: The Settings page UI — tab structure, typed controls per setting, save behavior, tags editor, version display, and the shared string-map editor component contract. + +### Modified Capabilities + +_None — backend config API behavior is unchanged; this is a frontend presentation change._ + +## Impact + +- **Frontend**: `src/routes/config/+page.svelte` (full rewrite), new shared map-editor component under `$lib/components/`, `SchemaForm.svelte` (delegate map editing to the shared component), `$lib/components/ui/field/` (new shadcn component). Uses already-installed `Tabs`, `Card`, `Switch`, `Input`, `InputGroup`. +- **Backend/APIs**: none — existing `GET/PUT /api/config` and `GET /api/version` are sufficient. +- **Behavior loss (accepted)**: unknown/legacy `app_config` keys are no longer visible or editable in the UI; they remain reachable via the API. diff --git a/openspec/changes/settings-page-redesign/specs/settings-page/spec.md b/openspec/changes/settings-page-redesign/specs/settings-page/spec.md new file mode 100644 index 0000000..75c3a74 --- /dev/null +++ b/openspec/changes/settings-page-redesign/specs/settings-page/spec.md @@ -0,0 +1,88 @@ +# Settings Page Delta + +## ADDED Requirements + +### Requirement: Tabbed Settings Layout +The Settings page SHALL organize settings into four tabs — General, +Default tags, Retention, and About — with General as the default tab. +General SHALL contain parallelism, terraform version, pack logs, and +SSH session logging. Retention SHALL contain run log retention and run +retention. About SHALL contain read-only version information (version, +commit, build date, Go version). + +#### Scenario: Landing on General +- **WHEN** a user navigates to the Settings page +- **THEN** the General tab is active, showing parallelism, terraform version, pack logs, and SSH session logging controls + +#### Scenario: About is read-only +- **WHEN** a user opens the About tab +- **THEN** version, commit, build date, and Go version are displayed with no editable controls + +### Requirement: Typed Controls with Human Labels +Each known setting SHALL render with a human-readable label and +description and a control matching its type: switches for booleans, +number inputs for integer settings, text inputs for strings. Raw +`app_config` keys SHALL NOT be used as labels. The system SHALL NOT +render settings through a generic key/value loop and SHALL NOT apply +sensitive-key masking. + +#### Scenario: Boolean is a switch +- **WHEN** the General tab renders the pack logs setting (`pack_logs_enabled`) +- **THEN** it is a switch with a human label, not a text input containing "true" + +#### Scenario: Unknown keys not rendered +- **WHEN** `app_config` contains a key not known to the frontend +- **THEN** the Settings page does not render a control for it + +### Requirement: Save Behavior +Switch settings SHALL persist immediately on toggle via +`PUT /api/config`. Tabs containing text or number inputs SHALL provide +a single Save action that persists only the dirty keys of that tab. +Failed saves SHALL surface an error without discarding the user's +edits. + +#### Scenario: Toggle saves immediately +- **WHEN** a user toggles SSH session logging +- **THEN** the value is persisted via `PUT /api/config` without a separate Save action + +#### Scenario: Per-tab save of dirty keys +- **WHEN** a user edits parallelism on the General tab and clicks Save +- **THEN** only the changed key is written; unchanged keys are not re-persisted + +#### Scenario: Save failure keeps edits +- **WHEN** a save request fails +- **THEN** an error is shown and the edited values remain in the form + +### Requirement: Retention Enable/Days Pairing +Each retention setting SHALL render as an enable switch paired with a +days input, and the days input SHALL be disabled while its switch is +off. + +#### Scenario: Days disabled when retention off +- **WHEN** run retention is toggled off +- **THEN** the run retention days input is disabled + +### Requirement: Default Tags Tab +The Default tags tab SHALL edit the org-wide `default_tags` setting +with a key/value editor and SHALL describe where the tags apply and +that packs can override individual tags. Blank keys or values SHALL be +rejected before saving. + +#### Scenario: Editing org tags +- **WHEN** a user adds `owner: secops` in the Default tags tab and saves +- **THEN** `PUT /api/config` persists `default_tags` as `{"owner": "secops"}` + +#### Scenario: Blank entry rejected client-side +- **WHEN** a user adds an entry with an empty key and saves +- **THEN** a validation message is shown and no request is sent + +### Requirement: Shared String-Map Editor +The key/value map editor SHALL be a single shared component used by +both the pack parameters dialog (via `SchemaForm`) and the Default tags +tab, preserving the existing editor behavior (add/remove entries, +blank-entry validation) and supporting read-only inherited rows for the +pack dialog's inheritance display. + +#### Scenario: One editor, two surfaces +- **WHEN** a user edits `default_tags` in the pack parameters dialog and in the Settings Default tags tab +- **THEN** both use the same key/value editor component with identical add/remove/validation behavior diff --git a/openspec/changes/settings-page-redesign/tasks.md b/openspec/changes/settings-page-redesign/tasks.md new file mode 100644 index 0000000..a41a8fd --- /dev/null +++ b/openspec/changes/settings-page-redesign/tasks.md @@ -0,0 +1,23 @@ +# Tasks: Settings Page Redesign + +## 1. Foundations + +- [ ] 1.1 Add the shadcn-svelte `field` component (`npx shadcn-svelte@latest add field`) +- [ ] 1.2 Extract the string-map editor from `SchemaForm.svelte` into a shared `KeyValueEditor` component (entries state, `objectToEntries`/`entriesToObject`, blank-entry validation, optional read-only inherited rows); wire `SchemaForm` to delegate to it +- [ ] 1.3 Verify pack parameters dialog behavior is unchanged after the extraction (add/remove/validate/save `default_tags`) +- [ ] 1.4 Align the frontend `AppConfig` TS type with the Go struct (all 8 keys + `default_tags`) + +## 2. Settings page rewrite + +- [ ] 2.1 Rebuild `src/routes/config/+page.svelte` with four tabs (General default, Default tags, Retention, About) using `Tabs` + `Field.FieldGroup`/`Field.Field`; delete the generic KV loop and sensitive-key masking +- [ ] 2.2 General tab: parallelism (number), terraform version (text), pack logs (switch), SSH session logging (switch); human labels + descriptions +- [ ] 2.3 Default tags tab: `KeyValueEditor` for `default_tags` with precedence description; blank entries rejected client-side +- [ ] 2.4 Retention tab: enable switch + days input pairs; days disabled while switch is off +- [ ] 2.5 About tab: version, commit, build date, Go version (read-only) +- [ ] 2.6 Save behavior: switches persist on toggle; per-tab Save writes only dirty keys; failed saves show an Alert and keep edits + +## 3. Verification + +- [ ] 3.1 `mise run build-frontend` and svelte-check pass +- [ ] 3.2 Manual flow: edit every setting type (switch, number, text, tags map), reload, confirm persistence; verify save-failure path keeps edits (e.g. invalid retention days rejected by server validation) +- [ ] 3.3 Verify pack parameters dialog still edits `default_tags` correctly through the shared editor diff --git a/openspec/specs/app-settings/spec.md b/openspec/specs/app-settings/spec.md new file mode 100644 index 0000000..f5ccc34 --- /dev/null +++ b/openspec/specs/app-settings/spec.md @@ -0,0 +1,65 @@ +# App Settings Specification + +## Purpose +Defines app-level admin settings stored in the `app_config` table and +carried by the existing `GET/PUT /api/config` endpoints. Currently +covers the org-wide `default_tags` setting: its storage shape, config +API validation, and how inherited tags are surfaced in the pack +parameters dialog. The runtime merge of org default tags into pack +parameters is specified in `pack-execution`. + +## Requirements + +### Requirement: Org-wide Default Tags Setting +The system SHALL store an org-wide default tags value as a string→string +map under the `default_tags` key in the `app_config` table, defaulting to +an empty map, exposed through the typed `AppConfig` as `DefaultTags`. The +key SHALL be backfilled by migration so it appears in `GET /api/config` +responses. + +#### Scenario: Default value present after migration +- **WHEN** the server starts against a database without a `default_tags` row +- **THEN** migrations create the row with value `{}` and `GET /api/config` includes `default_tags` + +#### Scenario: Survives pack lifecycle +- **WHEN** org default tags are set, and a pack is removed and reinstalled +- **THEN** the org default tags are unchanged and still apply to the reinstalled pack's detonations + +### Requirement: Default Tags Update Validation +`PUT /api/config` with key `default_tags` SHALL reject any value that is +not a JSON object whose values are all strings, returning HTTP 400 and +not persisting the value. Valid string→string objects (including the +empty object) SHALL be stored. + +#### Scenario: Non-object rejected +- **WHEN** a client sends `PUT /api/config` with key `default_tags` and value `"owner=secops"` +- **THEN** the server responds 400 and the stored value is unchanged + +#### Scenario: Non-string tag value rejected +- **WHEN** a client sends `PUT /api/config` with key `default_tags` and value `{"owner": 123}` +- **THEN** the server responds 400 and the stored value is unchanged + +#### Scenario: Valid map stored +- **WHEN** a client sends `PUT /api/config` with key `default_tags` and value `{"owner": "secops", "simulated": "true"}` +- **THEN** the server responds 204 and `GET /api/config` returns the new map + +### Requirement: Inherited Tags Visible in Pack Parameters Dialog +The pack parameters dialog SHALL display org-wide default tags as +read-only inherited entries within the `default_tags` field, visually +distinct from the pack's own entries and attributed to app settings. +When a pack-level entry uses the same key as an inherited entry, the +dialog SHALL indicate that the inherited value is overridden. Saving the +dialog SHALL persist only the pack's own entries to `packs.parameters`, +never the merged result. + +#### Scenario: Inherited tags shown read-only +- **WHEN** org default tags are `{"owner": "secops"}` and a user opens a pack's parameters dialog +- **THEN** `owner: secops` is shown as a non-editable inherited entry in the `default_tags` section + +#### Scenario: Override indicated +- **WHEN** org default tags contain `owner: secops` and the pack's own `default_tags` contain `owner: red-team` +- **THEN** the dialog indicates the inherited `owner` value is overridden by the pack-level entry + +#### Scenario: Merged values never persisted per-pack +- **WHEN** org default tags are `{"owner": "secops"}`, the pack's own `default_tags` are `{"team": "red"}`, and the user saves the dialog without edits +- **THEN** `packs.parameters.default_tags` contains only `{"team": "red"}` diff --git a/openspec/specs/pack-execution/spec.md b/openspec/specs/pack-execution/spec.md index 963c0aa..2e8d17c 100644 --- a/openspec/specs/pack-execution/spec.md +++ b/openspec/specs/pack-execution/spec.md @@ -64,6 +64,17 @@ top of any `default = ...` declared on the matching Terraform the pack's `params_schema` (so previously-stored unknown keys continue to flow until cleaned up). +For the `default_tags` key, org-wide default tags from app settings +SHALL be merged per-key beneath the pack-level value when pack +configurations are built from the database: an org tag applies unless +the pack's own `default_tags` sets the same key, and packs cannot +delete an org tag (no tombstones). The full precedence for a tag key is: +Terraform `variable` default < org-wide default tag < pack-level +`default_tags` entry < per-scenario `params`. An empty org map SHALL +leave pack parameters unchanged, and a pack-level `default_tags` value +that is not a string→string map SHALL pass through unmodified with no +org merge applied. + #### Scenario: Parameter as TF_VAR - **WHEN** the pack DB record has `parameters: {"region":"us-east-1"}` - **THEN** terraform inside the detonation runs with `TF_VAR_region=us-east-1` @@ -87,6 +98,23 @@ to flow until cleaned up). - **THEN** the key is still exported as a `TF_VAR_*` env var to the detonation's terraform process +#### Scenario: Org default tag applies to pack without own value +- **WHEN** org default tags are `{"owner": "secops"}` and the pack's + stored parameters have no `default_tags` key +- **THEN** the detonation's terraform runs with + `TF_VAR_default_tags={"owner":"secops"}` + +#### Scenario: Pack-level tag overrides org tag per-key +- **WHEN** org default tags are `{"owner": "secops", "env": "sim"}` and + the pack's `default_tags` are `{"owner": "red-team"}` +- **THEN** the effective map is `{"owner": "red-team", "env": "sim"}` + +#### Scenario: Empty org map is a no-op +- **WHEN** org default tags are `{}` and the pack's `default_tags` are + `{"team": "red"}` +- **THEN** the effective map is `{"team": "red"}`, identical to behavior + before org-wide default tags existed + ### Requirement: Conditional Terraform Lifecycle The system SHALL perform Terraform setup, apply, destroy only when the pack manifest's selected simulation contains a non-empty diff --git a/openspec/specs/packs/spec.md b/openspec/specs/packs/spec.md index b15d598..88194d2 100644 --- a/openspec/specs/packs/spec.md +++ b/openspec/specs/packs/spec.md @@ -37,55 +37,111 @@ persist a row for it. - **WHEN** a client posts POST /api/packs/install with type: "go-remote" - **THEN** the install is rejected with a validation error naming the allowed types (local, remote, upload) and no packs row is created +### Requirement: Install Derives Identity From Manifest +The system SHALL derive a pack's `name` and `version` from the binary's +`manifest` command (`pack.name` and `pack.version`) at install time, for all pack +types. The system SHALL ignore any `name` supplied in the install request. The +install SHALL run the manifest only after the binary is available (downloaded for +`remote`, present on disk for `local`, written to a temp location for `upload`), +and SHALL fail without creating a DB record if the manifest command fails. + +#### Scenario: Name comes from manifest, not request +- **WHEN** a client posts an install request whose body contains `name: "operator-typed"` and the resolved binary's manifest reports `pack.name: "real-pack"` +- **THEN** the persisted row's `name` is `real-pack` and the request's `name` is ignored + +#### Scenario: Version pinned from manifest or resolved tag +- **WHEN** an install completes successfully +- **THEN** the persisted `version` reflects the resolved release tag (remote) or the manifest's `pack.version` (local/upload), never an operator-typed value beyond an optional remote version pin + +#### Scenario: Manifest failure aborts install +- **WHEN** the `manifest` command fails for the resolved binary +- **THEN** the install fails with an error and no DB record is created + ### Requirement: Local Pack Install -The system SHALL accept `local` packs whose `source` is an absolute -filesystem path to an existing binary. The install endpoint SHALL NOT -copy the binary; it SHALL store the path as-is. Path validity SHALL be -verified at run time, not at install time. **Note:** an install can -succeed with a non-existent path; the failure surfaces only when a -scenario runs. +The system SHALL accept `local` packs whose `source` is an absolute filesystem +path to an existing binary. The install endpoint SHALL NOT copy the binary; it +SHALL reference the path as-is at run time. At install time the system SHALL +verify the path exists and SHALL run the binary's `manifest` command to derive +the pack's identity (see "Install Derives Identity From Manifest"). An install +SHALL fail if the path does not exist or the manifest command fails. #### Scenario: Path stored verbatim -- **WHEN** a client installs a local pack with `source: "/opt/packs/my-pack"` -- **THEN** the persisted row has `source = "/opt/packs/my-pack"` +- **WHEN** a client installs a local pack with `source: "/opt/packs/my-pack"` that exists and returns a valid manifest +- **THEN** the persisted row has `source = "/opt/packs/my-pack"` and the name/version come from the manifest -### Requirement: Remote Pack Install -The system SHALL fetch `remote` packs from GitHub releases at -`https://github.com///releases/download/v/___.tar.gz`, -verify the SHA-256 checksum against `checksums.txt` from the same -release, extract the binary, and cache it at -`/packs///`. +#### Scenario: Non-existent path rejected at install +- **WHEN** a client installs a local pack whose `source` path does not exist +- **THEN** the install fails with an error and no DB record is created -#### Scenario: Successful download -- **WHEN** a client installs `remote` pack `my-pack` v1.2.3 from `github.com/org/repo` -- **THEN** the binary is fetched, checksum-verified, and cached at `/packs/my-pack/1.2.3/my-pack` +### Requirement: Remote Pack Install +The system SHALL fetch `remote` packs through the GitHub Releases API. When +the request omits a version, the system SHALL resolve the release via +`GET /repos///releases/latest`; when a version is provided, via +`GET /repos///releases/tags/v`. The concrete resolved tag +SHALL be persisted as the pack's `version`. From the release's assets the system +SHALL select the archive whose name matches `*__.tar.gz`, fetch the +release's `checksums.txt` asset, verify the archive's SHA-256 against it, extract +the pack binary, and cache it at `/packs///` — where +`` is derived from the manifest (see "Install Derives Identity From +Manifest"). The in-tarball binary name SHALL be derived from the selected asset's +filename prefix; if the archive contains a single executable, that file SHALL be +used. The system SHALL NOT require the operator to supply the artifact filename +or the binary name. + +#### Scenario: Successful download of a pinned version +- **WHEN** a client installs a `remote` pack from `github.com/org/repo` with version `1.2.3` +- **THEN** the release at tag `v1.2.3` is resolved, the matching `*__.tar.gz` asset is checksum-verified and extracted, the binary is cached under `/packs//1.2.3/`, and the row's `version` is `1.2.3` + +#### Scenario: Latest resolution when version omitted +- **WHEN** a client installs a `remote` pack from `github.com/org/repo` with no version +- **THEN** the system resolves the latest release via the Releases API, persists the row's `version` as that release's concrete tag, and downloads that release's platform asset #### Scenario: Checksum mismatch - **WHEN** the downloaded archive's SHA-256 does not match the entry in `checksums.txt` - **THEN** the install fails with an error and no DB record is created +#### Scenario: No asset for the current platform +- **WHEN** the resolved release contains no asset matching `*__.tar.gz` +- **THEN** the install fails with an error naming the `/` and the resolved tag, and no DB record is created + +#### Scenario: Multiple matching assets +- **WHEN** the resolved release contains more than one asset matching `*__.tar.gz` +- **THEN** the install fails with an error listing the candidate asset names, and no DB record is created + +#### Scenario: Repository or release not found +- **WHEN** the GitHub Releases API returns not-found for the repo or the requested tag +- **THEN** the install fails with an error and no DB record is created + #### Scenario: Source format - **WHEN** a client provides `source: "https://github.com/org/repo"` or `"github.com/org/repo"` - **THEN** both forms are accepted (HTTP/HTTPS prefixes are stripped) ### Requirement: Upload Pack Install The system SHALL accept binary uploads via `POST /api/packs/upload` -(multipart form). The system SHALL write the binary to disk under -`/packs//upload/` and persist the `source` as that -path. Runtime SHALL treat `upload` packs identically to `local` packs. +(multipart form). The system SHALL write the binary to a temporary location, run +its `manifest` command to derive the pack's identity (see "Install Derives +Identity From Manifest"), relocate the binary to +`/packs//upload/`, and persist the `source` as that path. +Runtime SHALL treat `upload` packs identically to `local` packs. An install SHALL +fail if the manifest command fails, and no DB record SHALL be created. #### Scenario: Successful upload -- **WHEN** a client uploads a binary as multipart form data -- **THEN** the binary is written to `/packs//upload/` and a `packs` row is created with `type = "upload"` +- **WHEN** a client uploads a binary that returns a valid manifest as multipart form data +- **THEN** the binary is written to `/packs//upload/`, a `packs` row is created with `type = "upload"`, and the name/version come from the manifest + +#### Scenario: Manifest failure rejects upload +- **WHEN** an uploaded binary's `manifest` command fails +- **THEN** the install fails with an error and no DB record is created ### Requirement: Install Is Idempotent By Name -The system SHALL upsert by name on `POST /api/packs/install`. Subsequent -installs replace the existing row's type, source, and version. Cached +The system SHALL upsert by the manifest-derived `name` on `POST /api/packs/install` +and `POST /api/packs/upload`. Subsequent installs that resolve to the same +manifest `name` replace the existing row's type, source, and version. Cached binaries from previous installs SHALL NOT be cleaned up by the upsert. **Note:** flagged — disk usage grows when versions change repeatedly. #### Scenario: Reinstall replaces row -- **WHEN** a pack `p1` is installed at v1.0.0 then reinstalled at v2.0.0 +- **WHEN** a pack whose manifest reports name `p1` is installed at v1.0.0 then a newer release reporting the same name `p1` is installed at v2.0.0 - **THEN** the row's `version` is `2.0.0`, but the v1.0.0 binary remains under `/packs/p1/1.0.0/` ### Requirement: List Packs diff --git a/web/frontend/package-lock.json b/web/frontend/package-lock.json index 0640672..0a3b36b 100644 --- a/web/frontend/package-lock.json +++ b/web/frontend/package-lock.json @@ -1348,10 +1348,20 @@ } }, "node_modules/js-yaml": { - "version": "4.1.1", - "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.1.1.tgz", - "integrity": "sha512-qQKT4zQxXl8lLwBtHMWwaTcGfFOZviOJet3Oy/xmGk2gZH677CJM9EvtfdSkgWcATZhj/55JZ0rmy3myCT5lsA==", + "version": "4.3.0", + "resolved": "https://registry.npmjs.org/js-yaml/-/js-yaml-4.3.0.tgz", + "integrity": "sha512-1td788aAnnZ5qs7V2QIRl1owjtYpbKt749Y3xauqQgwIIGF/xXWz1wMTEBx5O3LK3lXLVuqXPdPxj2BoFHaW9Q==", "dev": true, + "funding": [ + { + "type": "github", + "url": "https://github.com/sponsors/puzrin" + }, + { + "type": "github", + "url": "https://github.com/sponsors/nodeca" + } + ], "license": "MIT", "dependencies": { "argparse": "^2.0.1" diff --git a/web/frontend/src/lib/api/client.ts b/web/frontend/src/lib/api/client.ts index b9142c8..1171fb0 100644 --- a/web/frontend/src/lib/api/client.ts +++ b/web/frontend/src/lib/api/client.ts @@ -187,7 +187,6 @@ export async function listPacks(): Promise { } export async function installPack(pack: { - name: string; type: string; source: string; version?: string; @@ -195,9 +194,8 @@ export async function installPack(pack: { return request('/packs/install', { method: 'POST', body: JSON.stringify(pack) }); } -export async function uploadPack(name: string, file: File): Promise { +export async function uploadPack(file: File): Promise { const formData = new FormData(); - formData.append('name', name); formData.append('file', file); const res = await fetch(`${BASE}/packs/upload`, { diff --git a/web/frontend/src/lib/components/PackCard.svelte b/web/frontend/src/lib/components/PackCard.svelte index 214ab4f..4918167 100644 --- a/web/frontend/src/lib/components/PackCard.svelte +++ b/web/frontend/src/lib/components/PackCard.svelte @@ -19,6 +19,7 @@ import Trash2Icon from '@lucide/svelte/icons/trash-2'; import ClockIcon from '@lucide/svelte/icons/clock'; import ArrowRightIcon from '@lucide/svelte/icons/arrow-right'; + import ExternalLinkIcon from '@lucide/svelte/icons/external-link'; let { pack, ondelete }: { pack: Pack; ondelete?: () => void } = $props(); @@ -66,6 +67,16 @@ } let displayVersion = $derived(manifest?.pack.version ?? pack.version); + + // Remote packs store their source as github.com/org/repo. Drop the redundant + // host so the card shows a scannable org/repo slug linking to the repo. + let remoteRepo = $derived.by(() => { + if (pack.type !== 'remote') return null; + const bare = pack.source.replace(/^https?:\/\//, ''); + const parts = bare.split('/'); + if (parts.length !== 3 || parts[0] !== 'github.com') return null; + return { slug: `${parts[1]}/${parts[2]}`, url: `https://${bare}` }; + }); let simulationCount = $derived(manifest?.simulations.length ?? null); let templateCount = $derived(manifest?.templates?.length ?? 0); @@ -131,9 +142,44 @@

{pack.name}

-

- {pack.type === 'upload' ? 'uploaded binary' : pack.source} -

+ {#if pack.type === 'upload'} +

uploaded binary

+ {:else if remoteRepo} + + + {#snippet child({ props })} + e.stopPropagation()} + class="mt-0.5 inline-flex max-w-full items-center gap-1 font-mono text-xs text-muted-foreground transition-colors hover:text-foreground" + > + {remoteRepo.slug} + + + {/snippet} + + {pack.source} + + {:else} + + + {#snippet child({ props })} + + {pack.source} + + {/snippet} + + {pack.source} + + {/if}
v{displayVersion} diff --git a/web/frontend/src/lib/components/PackParametersDialog.svelte b/web/frontend/src/lib/components/PackParametersDialog.svelte index 94acc0b..33ec4ad 100644 --- a/web/frontend/src/lib/components/PackParametersDialog.svelte +++ b/web/frontend/src/lib/components/PackParametersDialog.svelte @@ -3,13 +3,14 @@ import * as Dialog from '$lib/components/ui/dialog/index.js'; import { Button } from '$lib/components/ui/button/index.js'; import { + getConfig, getPackManifest, getPackParameters, updatePackParameters, ValidationError } from '$lib/api/client'; import SchemaForm from '$lib/components/SchemaForm.svelte'; - import type { PackManifest } from '$lib/types'; + import type { AppConfig, PackManifest } from '$lib/types'; let { open = $bindable(), @@ -30,6 +31,7 @@ let values = $state>({}); let schema = $state | undefined>(undefined); let unknownKeysFromServer = $state([]); + let orgDefaultTags = $state>({}); $effect(() => { if (open && packName) { @@ -42,12 +44,16 @@ error = ''; fieldErrors = {}; try { - const [params, manifest] = await Promise.all([ + const [params, manifest, cfg] = await Promise.all([ getPackParameters(packName), - getPackManifest(packName).catch(() => undefined as PackManifest | undefined) + getPackManifest(packName).catch(() => undefined as PackManifest | undefined), + // Inherited tags are display-only; a config fetch failure + // should not block editing pack parameters. + getConfig().catch(() => ({}) as AppConfig) ]); values = params; schema = manifest?.params_schema; + orgDefaultTags = extractStringMap(cfg['default_tags']); } catch (e) { error = e instanceof Error ? e.message : 'Failed to load parameters'; } finally { @@ -55,6 +61,17 @@ } } + // Org default_tags from app config; tolerates missing or malformed + // values (validation server-side may not have run on legacy rows). + function extractStringMap(v: unknown): Record { + if (!v || typeof v !== 'object' || Array.isArray(v)) return {}; + const out: Record = {}; + for (const [k, val] of Object.entries(v)) { + if (typeof val === 'string') out[k] = val; + } + return out; + } + // Reject string-map params (e.g. default_tags) that contain blank keys or // values. Terraform/cloud providers can't apply empty tag keys or values, // so saving them silently breaks every sim in the pack. Returns per-field @@ -125,7 +142,13 @@

Loading...

{:else}
- (values = next)} /> + (values = next)} + /> {#if unknownKeysFromServer.length > 0} diff --git a/web/frontend/src/lib/components/SchemaForm.svelte b/web/frontend/src/lib/components/SchemaForm.svelte index af24a27..84aab1b 100644 --- a/web/frontend/src/lib/components/SchemaForm.svelte +++ b/web/frontend/src/lib/components/SchemaForm.svelte @@ -29,12 +29,16 @@ // drives render order inside the section — regions first, then // the broad default_tags knob at the bottom. builtinNames = ['aws_region', 'gcp_region', 'azure_location', 'default_tags'], + // Org-wide default tags from app settings, shown read-only inside the + // default_tags section. Never written back through onchange. + inheritedDefaultTags = {}, onchange }: { schema: Record | undefined; values: Record; errors?: Record; builtinNames?: string[]; + inheritedDefaultTags?: Record; onchange: (next: Record) => void; } = $props(); @@ -67,9 +71,12 @@ let unknownKeys = $derived(Object.keys(values).filter((k) => !(k in properties))); // Auto-expand the cloud defaults section if any built-in has a saved - // value, otherwise start collapsed. + // value or org tags are inherited, otherwise start collapsed. // svelte-ignore state_referenced_locally - let cloudOpen = $state(builtinEntries.some(([name]) => values[name] !== undefined)); + let cloudOpen = $state( + builtinEntries.some(([name]) => values[name] !== undefined) || + Object.keys(inheritedDefaultTags).length > 0 + ); function update(name: string, value: unknown) { const next = { ...values, [name]: value }; @@ -152,7 +159,28 @@ {:else if prop.type === 'object' && prop.additionalProperties?.type === 'string'} {@const entries = objectToEntries(value)} + {@const inherited = name === 'default_tags' ? Object.entries(inheritedDefaultTags) : []}
+ {#if inherited.length > 0} + {@const packKeys = new Set(entries.map((e) => e.key.trim()))} +
+ {#each inherited as [k, v] (k)} + {@const overridden = packKeys.has(k)} +
+ + {k} + + + {v} + + {overridden ? 'overridden' : ''} +
+ {/each} +

Inherited from Settings

+
+ {/if} {#each entries as entry, i}
Install a new simulation pack.
-
@@ -149,8 +145,8 @@ {:else} - - + + {/if}