From 09e6683ff2b0edad770437316067a4a2a17335d0 Mon Sep 17 00:00:00 2001 From: agrasth Date: Mon, 29 Jun 2026 09:36:29 +0530 Subject: [PATCH 01/24] Add native RubyGems/Bundler command with auth injection and build-info Implements the core business logic for `jf ruby gem|bundle` native command: - RubyCommand dispatcher: routes to gem or bundle with stdio passthrough - Authentication injection: - Bundler: BUNDLE_ env var with proper host-key normalization - gem: GEM_HOST_API_KEY env var - Non-destructive, per-process only, respects existing native credentials - Host-match safety: skips injection when source host differs from server - Help bypass: no auth for help/-h/--help requests - Repository discovery: --source/--host args > Gemfile source > gem sources - Build-info collection: - Dependencies: via RubygemsFlexPack for bundle install/update/lock/add and opportunistic gem install/fetch - Artifacts: gem build (local .gem checksums) and gem push (+ property tagging) - AQL checksum enrichment: single batched query for all deps - 10 unit tests covering auth, repo extraction, help detection, host matching Note: go.mod contains local replace directive for build-info-go (development only, to be replaced with proper version bump at merge time). Co-authored-by: Cursor --- artifactory/commands/ruby/native_ruby.go | 764 ++++++++++++++++++ artifactory/commands/ruby/native_ruby_test.go | 120 +++ artifactory/commands/ruby/ruby.go | 46 +- go.mod | 2 + go.sum | 2 - 5 files changed, 926 insertions(+), 8 deletions(-) create mode 100644 artifactory/commands/ruby/native_ruby.go create mode 100644 artifactory/commands/ruby/native_ruby_test.go diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go new file mode 100644 index 00000000..8cd41b2a --- /dev/null +++ b/artifactory/commands/ruby/native_ruby.go @@ -0,0 +1,764 @@ +package ruby + +import ( + "bufio" + "encoding/json" + "fmt" + "io" + "net/url" + "os" + "os/exec" + "path/filepath" + "strings" + + buildinfo "github.com/jfrog/build-info-go/entities" + "github.com/jfrog/build-info-go/flexpack" + "github.com/jfrog/gofrog/crypto" + "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" + buildUtils "github.com/jfrog/jfrog-cli-core/v2/common/build" + coreConfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" + "github.com/jfrog/jfrog-client-go/artifactory/services" + specutils "github.com/jfrog/jfrog-client-go/artifactory/services/utils" + "github.com/jfrog/jfrog-client-go/auth" + "github.com/jfrog/jfrog-client-go/utils/log" +) + +// Supported native tools. +const ( + toolGem = "gem" + toolBundle = "bundle" +) + +// Run executes the native gem/bundle command with Artifactory auth injection and, +// when build parameters are supplied, collects build info. +func (rc *RubyCommand) Run() error { + if rc.nativeTool == "" { + rc.nativeTool = toolGem + } + if rc.nativeTool != toolGem && rc.nativeTool != toolBundle { + return fmt.Errorf("unsupported ruby tool %q: expected 'gem' or 'bundle'", rc.nativeTool) + } + + subCommand := "" + if len(rc.args) > 0 { + subCommand = rc.args[0] + } + + // Help requests must bypass auth injection entirely so credentials are never + // printed in help output (same rationale as the UV native command). + if isRubyHelpRequest(subCommand, rc.args) { + return runRubyBinary(rc.nativeTool, rc.args, nil) + } + + workingDir, err := os.Getwd() + if err != nil { + return fmt.Errorf("failed to get working directory: %w", err) + } + + serverDetails, srvErr := rc.ServerDetails() + if srvErr != nil { + log.Warn("Ruby auth: could not load jf server config — " + srvErr.Error()) + serverDetails = nil + } + + // Discover the Artifactory gem source the project points at, then inject auth. + sourceURL, repoKey := rc.resolveRepo(workingDir) + var extraEnv []string + if serverDetails != nil { + extraEnv = rc.injectAuth(serverDetails, sourceURL) + } + + log.Info(fmt.Sprintf("Running %s %s.", rc.nativeTool, subCommand)) + if runErr := runRubyBinary(rc.nativeTool, rc.args, extraEnv); runErr != nil { + return fmt.Errorf("%s %s failed: %w", rc.nativeTool, subCommand, runErr) + } + + if rc.buildConfiguration != nil { + buildName, nameErr := rc.buildConfiguration.GetBuildName() + if nameErr == nil && buildName != "" { + if biErr := rc.collectBuildInfo(workingDir, subCommand, repoKey, serverDetails); biErr != nil { + log.Warn("Failed to collect Ruby build info: " + biErr.Error()) + } + } + } + return nil +} + +// runRubyBinary executes gem/bundle with stdio pass-through and optional extra env vars. +func runRubyBinary(tool string, args, extraEnv []string) error { + cmd := exec.Command(tool, args...) // #nosec G204 -- tool is restricted to gem/bundle; args come from the user's own command line + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + if len(extraEnv) > 0 { + cmd.Env = append(os.Environ(), extraEnv...) + } + return cmd.Run() +} + +// isRubyHelpRequest reports whether the invocation is purely a help request. +func isRubyHelpRequest(subCommand string, args []string) bool { + if subCommand == "help" || subCommand == "" { + return true + } + for _, a := range args { + if a == "-h" || a == "--help" { + return true + } + } + return false +} + +// rubyResolveServerDetails resolves the jf server config for the given server ID, +// falling back to the default server when empty. +func rubyResolveServerDetails(serverID string) (*coreConfig.ServerDetails, error) { + if serverID == "" { + return coreConfig.GetDefaultServerConf() + } + return coreConfig.GetSpecificConfig(serverID, true, true) +} + +// ── Authentication ─────────────────────────────────────────────────────────── + +// injectAuth returns the additional environment variables required to authenticate +// the native tool against Artifactory. It is non-destructive: a credential is only +// injected when the user has not already configured one natively (env var, embedded +// URL credentials, ~/.gem/credentials, or .bundle/config), mirroring the UV flow. +// +// Bundler → BUNDLE_="user:password" (Bundler's per-host credential env var). +// RubyGems → GEM_HOST_API_KEY="user:password" (used by `gem push`/`gem fetch`). +func (rc *RubyCommand) injectAuth(serverDetails *coreConfig.ServerDetails, sourceURL string) []string { + user, pass := rubyCredentials(serverDetails) + if user == "" || pass == "" { + log.Debug("Ruby auth: no username/password/token available in server config; relying on native configuration") + return nil + } + + // Determine the host to authenticate. Prefer the discovered source URL host; + // otherwise fall back to the Artifactory server host. + host := rubyHostOf(sourceURL) + if host == "" { + host = rubyHostOf(serverDetails.ArtifactoryUrl) + } + // Without --server-id, only inject when the source host matches the jf server + // host to avoid leaking credentials to an unrelated registry. + if rc.serverID == "" && sourceURL != "" && !rubyHostMatchesServer(sourceURL, serverDetails.ArtifactoryUrl) { + log.Warn(fmt.Sprintf( + "Ruby auth: gem source host (%s) differs from jf server config host (%s) — "+ + "skipping credential injection. Use --server-id to authenticate explicitly, "+ + "or configure credentials with `bundle config set` / ~/.gem/credentials.", + host, rubyHostOf(serverDetails.ArtifactoryUrl))) + return nil + } + + var extraEnv []string + switch rc.nativeTool { + case toolBundle: + key := bundleEnvKeyForHost(host) + if os.Getenv(key) != "" { + log.Info(fmt.Sprintf("Ruby auth [bundle]: %s already set — respecting existing credentials", key)) + } else { + extraEnv = append(extraEnv, fmt.Sprintf("%s=%s:%s", key, user, pass)) + log.Info(fmt.Sprintf("Ruby auth [bundle]: injecting credentials via %s", key)) + } + case toolGem: + if os.Getenv("GEM_HOST_API_KEY") != "" { + log.Info("Ruby auth [gem]: GEM_HOST_API_KEY already set — respecting existing credentials") + } else { + extraEnv = append(extraEnv, fmt.Sprintf("GEM_HOST_API_KEY=%s:%s", user, pass)) + log.Info("Ruby auth [gem]: injecting credentials via GEM_HOST_API_KEY") + } + } + return extraEnv +} + +// rubyCredentials extracts the effective username/password, handling access tokens. +func rubyCredentials(serverDetails *coreConfig.ServerDetails) (user, pass string) { + user = serverDetails.GetUser() + pass = serverDetails.GetPassword() + if serverDetails.GetAccessToken() != "" { + if user == "" { + user = auth.ExtractUsernameFromAccessToken(serverDetails.GetAccessToken()) + } + pass = serverDetails.GetAccessToken() + } + return user, pass +} + +// bundleEnvKeyForHost converts a host into Bundler's per-host credential env var name, +// following Bundler's key normalization: uppercase, "." → "__", "-" → "___", and any +// remaining non-alphanumeric character → "_", prefixed with "BUNDLE_". +// +// "mycompany.jfrog.io" → "BUNDLE_MYCOMPANY__JFROG__IO" +func bundleEnvKeyForHost(host string) string { + key := strings.ToUpper(host) + key = strings.ReplaceAll(key, ".", "__") + key = strings.ReplaceAll(key, "-", "___") + var b strings.Builder + for _, r := range key { + switch { + case r >= 'A' && r <= 'Z', r >= '0' && r <= '9', r == '_': + b.WriteRune(r) + default: + b.WriteRune('_') + } + } + return "BUNDLE_" + b.String() +} + +// ── Repository discovery ─────────────────────────────────────────────────────── + +// resolveRepo discovers the Artifactory gem source URL and repo key the project uses. +// Precedence: explicit --repo override > --source/--host/--clear-sources arg > +// Gemfile `source` line > `gem sources` list. Returns empty strings when none is found. +func (rc *RubyCommand) resolveRepo(workingDir string) (sourceURL, repoKey string) { + if rc.repository != "" { + return "", rc.repository + } + // 1. Inspect the command args for an explicit source/host URL. + if u := rubySourceFromArgs(rc.args); u != "" { + return u, rubyExtractRepoKeyFromURL(u) + } + // 2. Gemfile `source ""` pointing at /api/gems/. + if u := rubySourceFromGemfile(workingDir); u != "" { + return u, rubyExtractRepoKeyFromURL(u) + } + // 3. Configured gem sources. + if u := rubySourceFromGemSources(); u != "" { + return u, rubyExtractRepoKeyFromURL(u) + } + return "", "" +} + +// rubySourceFromArgs returns the URL following --source/-s/--host/--clear-sources flags, +// or an inline "--source=" form. +func rubySourceFromArgs(args []string) string { + for i, a := range args { + switch { + case strings.HasPrefix(a, "--source="): + return strings.TrimPrefix(a, "--source=") + case strings.HasPrefix(a, "--host="): + return strings.TrimPrefix(a, "--host=") + case a == "--source" || a == "-s" || a == "--host": + if i+1 < len(args) { + return args[i+1] + } + } + } + return "" +} + +// rubySourceFromGemfile scans the project's Gemfile for a `source ""` directive +// that points at an Artifactory gems repository. +func rubySourceFromGemfile(workingDir string) string { + gemfile := filepath.Join(workingDir, "Gemfile") + data, err := os.ReadFile(gemfile) + if err != nil { + return "" + } + scanner := bufio.NewScanner(strings.NewReader(string(data))) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if !strings.HasPrefix(line, "source") { + continue + } + if u := extractQuotedURL(line); u != "" && strings.Contains(u, "/api/gems/") { + return u + } + } + return "" +} + +// rubySourceFromGemSources runs `gem sources --list` and returns the first Artifactory +// gems URL it finds. Best-effort: returns empty on any error. +func rubySourceFromGemSources() string { + out, err := exec.Command("gem", "sources", "--list").Output() + if err != nil { + return "" + } + scanner := bufio.NewScanner(strings.NewReader(string(out))) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if strings.Contains(line, "/api/gems/") && (strings.HasPrefix(line, "http://") || strings.HasPrefix(line, "https://")) { + return line + } + } + return "" +} + +// extractQuotedURL pulls the first single- or double-quoted token from a line. +func extractQuotedURL(line string) string { + for _, q := range []byte{'"', '\''} { + start := strings.IndexByte(line, q) + if start == -1 { + continue + } + end := strings.IndexByte(line[start+1:], q) + if end == -1 { + continue + } + return line[start+1 : start+1+end] + } + return "" +} + +// rubyExtractRepoKeyFromURL returns the repo key from a full Artifactory URL +// (".../api/gems//...") or returns the input unchanged when it is a bare key. +func rubyExtractRepoKeyFromURL(repoOrURL string) string { + if repoOrURL == "" { + return "" + } + if !strings.HasPrefix(repoOrURL, "http://") && !strings.HasPrefix(repoOrURL, "https://") { + return repoOrURL + } + parsed, err := url.Parse(repoOrURL) + if err != nil { + return "" + } + segments := strings.Split(strings.Trim(parsed.Path, "/"), "/") + for i, seg := range segments { + if seg == "gems" && i+1 < len(segments) { + return segments[i+1] + } + // Also handle "/api/gems/". + if seg == "api" && i+2 < len(segments) && segments[i+1] == "gems" { + return segments[i+2] + } + } + return "" +} + +// rubyHostOf returns the host[:port] of a URL, or "" when not parseable. +func rubyHostOf(rawURL string) string { + if rawURL == "" { + return "" + } + parsed, err := url.Parse(rawURL) + if err != nil { + return "" + } + return parsed.Host +} + +// rubyHostMatchesServer reports whether rawURL has the same host as the Artifactory URL. +func rubyHostMatchesServer(rawURL, artifactoryURL string) bool { + h := rubyHostOf(rawURL) + return h != "" && h == rubyHostOf(artifactoryURL) +} + +// ── Build info ───────────────────────────────────────────────────────────────── + +// collectBuildInfo dispatches build-info collection based on the native tool/sub-command. +func (rc *RubyCommand) collectBuildInfo(workingDir, subCommand, repoKey string, serverDetails *coreConfig.ServerDetails) error { + switch { + case rc.nativeTool == toolGem && (subCommand == "build" || subCommand == "push"): + return rc.collectGemArtifactBuildInfo(workingDir, subCommand, repoKey, serverDetails) + case rc.collectsDependencies(subCommand): + return rc.collectDependencyBuildInfo(workingDir, subCommand, repoKey, serverDetails) + default: + log.Debug(fmt.Sprintf("Ruby build-info: no collection for '%s %s'", rc.nativeTool, subCommand)) + return nil + } +} + +// collectsDependencies reports whether the sub-command resolves a dependency tree +// (i.e. produces/uses a Gemfile.lock we can read). +func (rc *RubyCommand) collectsDependencies(subCommand string) bool { + if rc.nativeTool == toolBundle { + switch subCommand { + case "install", "update", "lock", "add": + return true + } + } + if rc.nativeTool == toolGem { + // `gem install`/`gem fetch` only yield a Gemfile.lock-style tree inside a + // bundler project; collected opportunistically when a lock file exists. + switch subCommand { + case "install", "fetch": + return true + } + } + return false +} + +// collectDependencyBuildInfo parses Gemfile.lock and records dependencies, enriching +// checksums from Artifactory. +func (rc *RubyCommand) collectDependencyBuildInfo(workingDir, subCommand, repoKey string, serverDetails *coreConfig.ServerDetails) error { + buildName, err := rc.buildConfiguration.GetBuildName() + if err != nil { + return err + } + buildNumber, err := rc.buildConfiguration.GetBuildNumber() + if err != nil { + return err + } + + gemConfig := flexpack.GemConfig{WorkingDirectory: workingDir} + // For bundler, use `bundle list` as the ground-truth installed set so group + // filtering (--without/--with) is reflected accurately. + if rc.nativeTool == toolBundle { + gemConfig.InstalledPackages = bundleInstalledPackages(workingDir) + } + + collector, err := flexpack.NewRubygemsFlexPack(gemConfig) + if err != nil { + return fmt.Errorf("failed to create RubyGems FlexPack collector: %w", err) + } + bi, err := collector.CollectBuildInfo(buildName, buildNumber) + if err != nil { + return fmt.Errorf("failed to collect RubyGems build info: %w", err) + } + + if customModule := rc.buildConfiguration.GetModule(); customModule != "" && len(bi.Modules) > 0 { + bi.Modules[0].Id = customModule + } + + if len(bi.Modules) > 0 && len(bi.Modules[0].Dependencies) > 0 && repoKey != "" && serverDetails != nil { + directURLDeps := collector.GetDirectURLDeps() + rubyEnrichDepsFromArtifactory(bi.Modules[0].Dependencies, repoKey, directURLDeps, serverDetails) + } else if repoKey == "" { + log.Info("Ruby build-info: no Artifactory gems repo discovered — dependency checksum enrichment skipped. " + + "Point your Gemfile/gem source at an Artifactory gems repository or pass --server-id.") + } + + if err := rubySaveBuildInfo(bi, rc.buildConfiguration); err != nil { + return fmt.Errorf("failed to save RubyGems build info: %w", err) + } + log.Info(fmt.Sprintf("RubyGems build info collected. Use 'jf rt bp %s %s' to publish.", buildName, buildNumber)) + return nil +} + +// collectGemArtifactBuildInfo records the .gem artifact produced by `gem build`/`gem push`. +func (rc *RubyCommand) collectGemArtifactBuildInfo(workingDir, subCommand, repoKey string, serverDetails *coreConfig.ServerDetails) error { + buildName, err := rc.buildConfiguration.GetBuildName() + if err != nil { + return err + } + buildNumber, err := rc.buildConfiguration.GetBuildNumber() + if err != nil { + return err + } + + artifacts, err := rubyCollectGemArtifacts(workingDir, rc.args) + if err != nil { + return fmt.Errorf("failed to collect gem artifacts: %w", err) + } + if len(artifacts) == 0 { + log.Debug("Ruby build-info: no .gem artifacts found to record") + return nil + } + + moduleID := rc.gemModuleID(workingDir) + if customModule := rc.buildConfiguration.GetModule(); customModule != "" { + moduleID = customModule + } + + bi := &buildinfo.BuildInfo{ + Name: buildName, + Number: buildNumber, + Agent: &buildinfo.Agent{Name: "gem"}, + BuildAgent: &buildinfo.Agent{Name: "Generic", Version: "1.0"}, + Modules: []buildinfo.Module{{ + Id: moduleID, + Type: buildinfo.Gem, + Artifacts: artifacts, + }}, + } + + // On push, set build properties on the uploaded artifacts in Artifactory. + if subCommand == "push" && repoKey != "" && serverDetails != nil { + if propErr := rubySetBuildProperties(serverDetails, repoKey, buildName, buildNumber, rc.buildConfiguration.GetProject(), bi); propErr != nil { + log.Warn("Failed to set build properties on gem artifacts: " + propErr.Error()) + } + } + + if err := rubySaveBuildInfo(bi, rc.buildConfiguration); err != nil { + return fmt.Errorf("failed to save RubyGems build info: %w", err) + } + log.Info(fmt.Sprintf("RubyGems build info collected. Use 'jf rt bp %s %s' to publish.", buildName, buildNumber)) + return nil +} + +// gemModuleID derives a module ID for gem build/push from the gemspec/dir name. +func (rc *RubyCommand) gemModuleID(workingDir string) string { + name := filepath.Base(workingDir) + if name == "" || name == "." || name == string(filepath.Separator) { + return "ruby-project" + } + return name +} + +// rubyCollectGemArtifacts locates .gem files (build output or explicit push target) +// and computes their checksums for build-info. +func rubyCollectGemArtifacts(workingDir string, args []string) ([]buildinfo.Artifact, error) { + gemFiles := make(map[string]bool) + + // Explicit .gem path on a `gem push ` command. + for _, a := range args { + if strings.HasSuffix(a, ".gem") { + p := a + if !filepath.IsAbs(p) { + p = filepath.Join(workingDir, a) + } + gemFiles[p] = true + } + } + + // `gem build` writes -.gem into the working dir and pkg/. + for _, dir := range []string{workingDir, filepath.Join(workingDir, "pkg")} { + entries, err := os.ReadDir(dir) + if err != nil { + continue + } + for _, e := range entries { + if !e.IsDir() && strings.HasSuffix(e.Name(), ".gem") { + gemFiles[filepath.Join(dir, e.Name())] = true + } + } + } + + var artifacts []buildinfo.Artifact + for path := range gemFiles { + checksum, err := rubyFileChecksums(path) + if err != nil { + log.Warn(fmt.Sprintf("Could not compute checksums for %s: %v", path, err)) + continue + } + artifacts = append(artifacts, buildinfo.Artifact{ + Name: filepath.Base(path), + Type: gemDepArtifactType, + Path: filepath.Base(path), + Checksum: checksum, + }) + } + return artifacts, nil +} + +// rubyFileChecksums calculates SHA1, SHA256 and MD5 for a file. +func rubyFileChecksums(filePath string) (buildinfo.Checksum, error) { + fileDetails, err := crypto.GetFileDetails(filePath, true) + if err != nil { + return buildinfo.Checksum{}, fmt.Errorf("failed to calculate checksums: %w", err) + } + return buildinfo.Checksum{ + Sha1: fileDetails.Checksum.Sha1, + Sha256: fileDetails.Checksum.Sha256, + Md5: fileDetails.Checksum.Md5, + }, nil +} + +// gemDepArtifactType is the build-info artifact/dependency type for gem files. +const gemDepArtifactType = "gem" + +// rubySaveBuildInfo persists the build info locally for a later `jf rt bp`. +func rubySaveBuildInfo(bi *buildinfo.BuildInfo, buildConfiguration *buildUtils.BuildConfiguration) error { + service := buildUtils.CreateBuildInfoService() + bld, err := service.GetOrCreateBuildWithProject(bi.Name, bi.Number, buildConfiguration.GetProject()) + if err != nil { + return fmt.Errorf("failed to create build: %w", err) + } + return bld.SaveBuildInfo(bi) +} + +// bundleInstalledPackages runs `bundle list` and returns the installed gems as +// name → version. Returns nil on error (caller falls back to including the full lock). +func bundleInstalledPackages(workingDir string) map[string]string { + cmd := exec.Command("bundle", "list") + cmd.Dir = workingDir + out, err := cmd.Output() + if err != nil { + log.Debug(fmt.Sprintf("bundle list failed, using full Gemfile.lock for build-info: %v", err)) + return nil + } + installed := make(map[string]string) + scanner := bufio.NewScanner(strings.NewReader(string(out))) + for scanner.Scan() { + // Lines look like: " * rake (13.0.6)" + line := strings.TrimSpace(scanner.Text()) + line = strings.TrimPrefix(line, "* ") + name, version := parseBundleListLine(line) + if name != "" { + installed[name] = version + } + } + if len(installed) == 0 { + return nil + } + return installed +} + +// parseBundleListLine parses "rake (13.0.6)" → name, version. +func parseBundleListLine(line string) (name, version string) { + open := strings.Index(line, " (") + if open == -1 { + return "", "" + } + name = strings.TrimSpace(line[:open]) + rest := line[open+2:] + if closeIdx := strings.IndexByte(rest, ')'); closeIdx != -1 { + version = strings.TrimSpace(rest[:closeIdx]) + } + return name, version +} + +// rubyEnrichDepsFromArtifactory fetches sha1/sha256/md5 for registry-based dependencies +// in a single batched AQL call, matching .gem filenames by "-" prefix. +// GIT/PATH deps (in directURLDeps) are skipped since they are not stored in Artifactory. +func rubyEnrichDepsFromArtifactory(deps []buildinfo.Dependency, repoKey string, directURLDeps map[string]string, serverDetails *coreConfig.ServerDetails) { + if len(deps) == 0 { + return + } + servicesManager, err := utils.CreateServiceManager(serverDetails, -1, 0, false) + if err != nil { + log.Warn("Could not create services manager for dependency enrichment: " + err.Error()) + return + } + searchRepo, err := utils.GetRepoNameForDependenciesSearch(repoKey, servicesManager) + if err != nil { + log.Warn("Could not resolve repo for dependency search, using as-is: " + err.Error()) + searchRepo = repoKey + } + + type depEntry struct { + idx int + prefix string // "-" used to match the .gem filename + } + var entries []depEntry + for i, dep := range deps { + if dep.Id == "" { + continue + } + if _, isDirect := directURLDeps[dep.Id]; isDirect { + continue + } + colonIdx := strings.LastIndex(dep.Id, ":") + if colonIdx < 0 { + continue + } + name, version := dep.Id[:colonIdx], dep.Id[colonIdx+1:] + entries = append(entries, depEntry{i, name + "-" + version}) + } + if len(entries) == 0 { + return + } + + var orClauses []string + seen := make(map[string]bool) + for _, e := range entries { + if seen[e.prefix] { + continue + } + seen[e.prefix] = true + // Match "-.gem" and platform-specific "--.gem". + orClauses = append(orClauses, fmt.Sprintf(`{"name":{"$match":%q}}`, e.prefix+"*.gem")) + } + aqlQuery := fmt.Sprintf( + `items.find({"repo":%q,"$or":[%s]}).include("name","actual_sha1","actual_md5","sha256")`, + searchRepo, strings.Join(orClauses, ","), + ) + + stream, err := servicesManager.Aql(aqlQuery) + if err != nil { + log.Debug(fmt.Sprintf("Batch AQL enrichment failed for repo %s: %v", searchRepo, err)) + return + } + raw, _ := io.ReadAll(stream) + _ = stream.Close() + + var aqlResult struct { + Results []struct { + Name string `json:"name"` + ActualSha1 string `json:"actual_sha1"` + ActualMd5 string `json:"actual_md5"` + Sha256 string `json:"sha256"` + } `json:"results"` + } + if err := json.Unmarshal(raw, &aqlResult); err != nil { + log.Debug(fmt.Sprintf("Failed to parse AQL enrichment response: %v", err)) + return + } + + enriched := 0 + for _, r := range aqlResult.Results { + if r.ActualSha1 == "" { + continue + } + for _, e := range entries { + if deps[e.idx].Sha1 != "" { + continue + } + // "-.gem" or "--.gem" + if r.Name == e.prefix+".gem" || strings.HasPrefix(r.Name, e.prefix+"-") { + deps[e.idx].Sha1 = r.ActualSha1 + deps[e.idx].Md5 = r.ActualMd5 + if r.Sha256 != "" && deps[e.idx].Sha256 == "" { + deps[e.idx].Sha256 = r.Sha256 + } + enriched++ + break + } + } + } + + if enriched > 0 { + log.Info(fmt.Sprintf("Enriched %d/%d RubyGems dependencies with Artifactory checksums (repo: %s)", enriched, len(deps), searchRepo)) + } else { + log.Debug(fmt.Sprintf("No RubyGems dependencies enriched from repo %s — gems may not be cached yet", searchRepo)) + } +} + +// rubyAqlQueryForSearch builds an AQL ItemsFind expression matching a file by name. +func rubyAqlQueryForSearch(repo, file string) string { + return fmt.Sprintf( + `{"repo": %q, "$or": [{"$and": [{"path": {"$match": "*"}, "name": {"$match": %q}}]}]}`, + repo, file, + ) +} + +// rubySetBuildProperties tags uploaded .gem artifacts with build.name/number properties +// so they are linked to the build in Artifactory. +func rubySetBuildProperties(serverDetails *coreConfig.ServerDetails, repoKey, buildName, buildNumber, project string, bi *buildinfo.BuildInfo) error { + servicesManager, err := utils.CreateServiceManager(serverDetails, -1, 0, false) + if err != nil { + return fmt.Errorf("failed to create services manager: %w", err) + } + searchRepo, err := utils.GetRepoNameForDependenciesSearch(repoKey, servicesManager) + if err != nil { + searchRepo = repoKey + } + + if err := buildUtils.SaveBuildGeneralDetails(buildName, buildNumber, project); err != nil { + return fmt.Errorf("SaveBuildGeneralDetails failed: %w", err) + } + buildProps, err := buildUtils.CreateBuildProperties(buildName, buildNumber, project) + if err != nil { + return fmt.Errorf("CreateBuildProperties failed: %w", err) + } + + if len(bi.Modules) == 0 || len(bi.Modules[0].Artifacts) == 0 { + return nil + } + for _, artifact := range bi.Modules[0].Artifacts { + searchParams := services.SearchParams{ + CommonParams: &specutils.CommonParams{ + Aql: specutils.Aql{ + ItemsFind: rubyAqlQueryForSearch(searchRepo, artifact.Name), + }, + }, + } + searchReader, searchErr := servicesManager.SearchFiles(searchParams) + if searchErr != nil { + log.Warn(fmt.Sprintf("Failed to find artifact %s: %v", artifact.Name, searchErr)) + continue + } + _, setErr := servicesManager.SetProps(services.PropsParams{Reader: searchReader, Props: buildProps}) + if closeErr := searchReader.Close(); closeErr != nil { + log.Warn("Failed to close search reader:", closeErr) + } + if setErr != nil { + log.Warn(fmt.Sprintf("Failed to set properties on artifact %s: %v", artifact.Name, setErr)) + } + } + log.Info(fmt.Sprintf("Successfully set build properties on %d artifacts", len(bi.Modules[0].Artifacts))) + return nil +} diff --git a/artifactory/commands/ruby/native_ruby_test.go b/artifactory/commands/ruby/native_ruby_test.go new file mode 100644 index 00000000..8894e624 --- /dev/null +++ b/artifactory/commands/ruby/native_ruby_test.go @@ -0,0 +1,120 @@ +package ruby + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBundleEnvKeyForHost(t *testing.T) { + cases := []struct { + host string + want string + }{ + {"mycompany.jfrog.io", "BUNDLE_MYCOMPANY__JFROG__IO"}, + {"my-art.example.com", "BUNDLE_MY___ART__EXAMPLE__COM"}, + {"localhost:8081", "BUNDLE_LOCALHOST_8081"}, + {"artifactory", "BUNDLE_ARTIFACTORY"}, + } + for _, c := range cases { + assert.Equal(t, c.want, bundleEnvKeyForHost(c.host), "host %q", c.host) + } +} + +func TestRubyExtractRepoKeyFromURL(t *testing.T) { + cases := []struct { + in string + want string + }{ + {"https://my.jfrog.io/artifactory/api/gems/gems-local/", "gems-local"}, + {"https://my.jfrog.io/api/gems/gems-remote", "gems-remote"}, + {"gems-local", "gems-local"}, // bare key passthrough + {"https://rubygems.org/", ""}, // no /api/gems/ segment + {"", ""}, + } + for _, c := range cases { + assert.Equal(t, c.want, rubyExtractRepoKeyFromURL(c.in), "input %q", c.in) + } +} + +func TestRubySourceFromArgs(t *testing.T) { + assert.Equal(t, "https://h/api/gems/r/", rubySourceFromArgs([]string{"install", "--source", "https://h/api/gems/r/"})) + assert.Equal(t, "https://h/api/gems/r/", rubySourceFromArgs([]string{"push", "x.gem", "--host=https://h/api/gems/r/"})) + assert.Equal(t, "https://h/api/gems/r/", rubySourceFromArgs([]string{"install", "-s", "https://h/api/gems/r/"})) + assert.Equal(t, "", rubySourceFromArgs([]string{"install", "rake"})) +} + +func TestRubySourceFromGemfile(t *testing.T) { + dir := t.TempDir() + gemfile := `source "https://rubygems.org" +source 'https://my.jfrog.io/artifactory/api/gems/gems-virtual/' + +gem "rails" +` + if err := os.WriteFile(filepath.Join(dir, "Gemfile"), []byte(gemfile), 0644); err != nil { + t.Fatal(err) + } + assert.Equal(t, "https://my.jfrog.io/artifactory/api/gems/gems-virtual/", rubySourceFromGemfile(dir)) + + // No artifactory source → empty. + empty := t.TempDir() + _ = os.WriteFile(filepath.Join(empty, "Gemfile"), []byte(`source "https://rubygems.org"`), 0644) + assert.Equal(t, "", rubySourceFromGemfile(empty)) + + // No Gemfile → empty. + assert.Equal(t, "", rubySourceFromGemfile(t.TempDir())) +} + +func TestIsRubyHelpRequest(t *testing.T) { + assert.True(t, isRubyHelpRequest("help", []string{"help"})) + assert.True(t, isRubyHelpRequest("", nil)) + assert.True(t, isRubyHelpRequest("install", []string{"install", "--help"})) + assert.True(t, isRubyHelpRequest("install", []string{"install", "-h"})) + assert.False(t, isRubyHelpRequest("install", []string{"install", "rake"})) +} + +func TestParseBundleListLine(t *testing.T) { + name, version := parseBundleListLine("rake (13.0.6)") + assert.Equal(t, "rake", name) + assert.Equal(t, "13.0.6", version) + + name, version = parseBundleListLine("nokogiri (1.13.9-x86_64-linux)") + assert.Equal(t, "nokogiri", name) + assert.Equal(t, "1.13.9-x86_64-linux", version) + + name, _ = parseBundleListLine("Gems included by the bundle:") + assert.Equal(t, "", name) +} + +func TestExtractQuotedURL(t *testing.T) { + assert.Equal(t, "https://x/y", extractQuotedURL(`source "https://x/y"`)) + assert.Equal(t, "https://x/y", extractQuotedURL(`source 'https://x/y'`)) + assert.Equal(t, "", extractQuotedURL(`source https://x/y`)) +} + +func TestRubyCommandSettersAndName(t *testing.T) { + cmd := NewRubyCommand(). + SetNativeTool("bundle"). + SetArgs([]string{"install"}). + SetServerID("my-server"). + SetRepo("gems-local") + assert.Equal(t, "rt_ruby_native", cmd.CommandName()) + assert.Equal(t, "bundle", cmd.nativeTool) + assert.Equal(t, "gems-local", cmd.repository) + assert.Equal(t, "my-server", cmd.serverID) +} + +func TestRubyHostMatchesServer(t *testing.T) { + assert.True(t, rubyHostMatchesServer("https://my.jfrog.io/artifactory/api/gems/r/", "https://my.jfrog.io/artifactory")) + assert.False(t, rubyHostMatchesServer("https://other.com/api/gems/r/", "https://my.jfrog.io/artifactory")) + assert.False(t, rubyHostMatchesServer("", "https://my.jfrog.io/artifactory")) +} + +func TestRubyRunUnsupportedTool(t *testing.T) { + cmd := NewRubyCommand().SetNativeTool("npm").SetArgs([]string{"install"}) + err := cmd.Run() + assert.Error(t, err) + assert.Contains(t, err.Error(), "unsupported ruby tool") +} diff --git a/artifactory/commands/ruby/ruby.go b/artifactory/commands/ruby/ruby.go index a24cdc8b..d60f230d 100644 --- a/artifactory/commands/ruby/ruby.go +++ b/artifactory/commands/ruby/ruby.go @@ -3,22 +3,43 @@ package ruby import ( "net/url" + buildUtils "github.com/jfrog/jfrog-cli-core/v2/common/build" "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/jfrog/jfrog-client-go/auth" "github.com/jfrog/jfrog-client-go/utils/errorutils" ) +// RubyCommand runs a native RubyGems (`gem`) or Bundler (`bundle`) command directly, +// injecting Artifactory authentication and optionally collecting build info. +// +// It is a config-less native flow (like `jf uv`): the server is resolved from +// --server-id or the default server, and the Artifactory repository is discovered +// from the project's own Ruby configuration (Gemfile source, .bundle/config, +// gem sources). The `.jfrog/projects/ruby.yaml` produced by `jf ruby-config` is NOT +// read by this command. type RubyCommand struct { serverDetails *config.ServerDetails - commandName string - args []string - repository string + // nativeTool is the underlying binary to run: "gem" or "bundle". + nativeTool string + // args are the arguments passed to the native tool, including its sub-command + // (e.g. ["install", "--without", "test"]). + args []string + // serverID is the explicit --server-id, empty for the default server. + serverID string + // repository optionally overrides the Artifactory repo (otherwise auto-discovered). + repository string + buildConfiguration *buildUtils.BuildConfiguration } func NewRubyCommand() *RubyCommand { return &RubyCommand{} } +func (rc *RubyCommand) SetNativeTool(tool string) *RubyCommand { + rc.nativeTool = tool + return rc +} + func (rc *RubyCommand) SetRepo(repo string) *RubyCommand { rc.repository = repo return rc @@ -29,8 +50,13 @@ func (rc *RubyCommand) SetArgs(arguments []string) *RubyCommand { return rc } -func (rc *RubyCommand) SetCommandName(commandName string) *RubyCommand { - rc.commandName = commandName +func (rc *RubyCommand) SetServerID(serverID string) *RubyCommand { + rc.serverID = serverID + return rc +} + +func (rc *RubyCommand) SetBuildConfiguration(bc *buildUtils.BuildConfiguration) *RubyCommand { + rc.buildConfiguration = bc return rc } @@ -40,7 +66,15 @@ func (rc *RubyCommand) SetServerDetails(serverDetails *config.ServerDetails) *Ru } func (rc *RubyCommand) ServerDetails() (*config.ServerDetails, error) { - return rc.serverDetails, nil + if rc.serverDetails != nil { + return rc.serverDetails, nil + } + return rubyResolveServerDetails(rc.serverID) +} + +// CommandName is the usage-report metric id for native Ruby commands. +func (rc *RubyCommand) CommandName() string { + return "rt_ruby_native" } // GetRubyGemsRepoUrlWithCredentials gets the RubyGems repository url and the credentials. diff --git a/go.mod b/go.mod index 118c7b34..a1f697f8 100644 --- a/go.mod +++ b/go.mod @@ -198,6 +198,8 @@ require ( sigs.k8s.io/yaml v1.6.0 // indirect ) +replace github.com/jfrog/build-info-go => ../build-info-go + // replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260604085947-7c110b77b4b4 // replace github.com/gfleury/go-bitbucket-v1 => github.com/gfleury/go-bitbucket-v1 v0.0.0-20230825095122-9bc1711434ab diff --git a/go.sum b/go.sum index 359c541c..fd95ec58 100644 --- a/go.sum +++ b/go.sum @@ -378,8 +378,6 @@ github.com/jellydator/ttlcache/v3 v3.4.0 h1:YS4P125qQS0tNhtL6aeYkheEaB/m8HCqdMMP github.com/jellydator/ttlcache/v3 v3.4.0/go.mod h1:Hw9EgjymziQD3yGsQdf1FqFdpp7YjFMd4Srg5EJlgD4= github.com/jfrog/archiver/v3 v3.6.3 h1:hkAmPjBw393tPmQ07JknLNWFNZjXdy2xFEnOW9wwOxI= github.com/jfrog/archiver/v3 v3.6.3/go.mod h1:5V9l+Fte30Y4qe9dUOAd3yNTf8lmtVNuhKNrvI8PMhg= -github.com/jfrog/build-info-go v1.13.1-0.20260610071651-260ad6720e0d h1:34G3TEVZfbpAFqAt/BiXrS4dA8vZfofkdW7qCQAYSgM= -github.com/jfrog/build-info-go v1.13.1-0.20260610071651-260ad6720e0d/go.mod h1:CYRUCvLKfyARjoJXLWAxce1qNUxTEtbRKAARkV42vpE= github.com/jfrog/froggit-go v1.21.1 h1:I/XUOO6GQ1d/rmBlM361F8T654C3ohIWrpw23xNL9JY= github.com/jfrog/froggit-go v1.21.1/go.mod h1:umBiakJB0CSPFfe0AHVaC3n9xsmUT7NGkDCny3bRchI= github.com/jfrog/gofrog v1.7.6 h1:QmfAiRzVyaI7JYGsB7cxfAJePAZTzFz0gRWZSE27c6s= From 5a2d362cf50c0ce9afa43e6ef71604c215b90cfe Mon Sep 17 00:00:00 2001 From: agrasth Date: Mon, 29 Jun 2026 11:52:08 +0530 Subject: [PATCH 02/24] fix: auth bugs + add --repo flag for URL construction Bug fixes: - Bug 1: GEM_HOST_API_KEY now uses Basic base64 encoding (was raw user:pass which Artifactory rejected) - Bug 2: gem install/fetch embeds credentials in --source URL for index downloads (specs.4.8.gz) since GEM_HOST_API_KEY is not used for those requests in RubyGems 3.x - Bug 3: explicit no-args error before help bypass prevents silent fallthrough to gem help - Bug 4: skip credential injection when no Artifactory source is discovered (even with --server-id) New feature: - --repo flag constructs the full Artifactory gems API URL from server config + repo name, eliminating the need to pass full URLs. For gem install/push, injects --source/--host automatically. Also enriches dependency repo path from AQL results in build-info. Co-authored-by: Cursor --- artifactory/commands/ruby/native_ruby.go | 142 ++++++++++++++++-- artifactory/commands/ruby/native_ruby_test.go | 95 +++++++++++- 2 files changed, 223 insertions(+), 14 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index 8cd41b2a..bef3e3ec 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -2,6 +2,7 @@ package ruby import ( "bufio" + "encoding/base64" "encoding/json" "fmt" "io" @@ -39,12 +40,15 @@ func (rc *RubyCommand) Run() error { return fmt.Errorf("unsupported ruby tool %q: expected 'gem' or 'bundle'", rc.nativeTool) } - subCommand := "" - if len(rc.args) > 0 { - subCommand = rc.args[0] + // Bug 3 fix: explicit no-args check before help bypass so we don't + // silently fall into gem help when no subcommand is given. + if len(rc.args) == 0 { + return fmt.Errorf("no subcommand provided for '%s'. Usage: jf ruby %s [args...]", rc.nativeTool, rc.nativeTool) } - // Help requests must bypass auth injection entirely so credentials are never + subCommand := rc.args[0] + + // Help requests bypass auth injection entirely so credentials are never // printed in help output (same rationale as the UV native command). if isRubyHelpRequest(subCommand, rc.args) { return runRubyBinary(rc.nativeTool, rc.args, nil) @@ -62,10 +66,24 @@ func (rc *RubyCommand) Run() error { } // Discover the Artifactory gem source the project points at, then inject auth. - sourceURL, repoKey := rc.resolveRepo(workingDir) + sourceURL, repoKey := rc.resolveRepo(workingDir, serverDetails) + + // When --repo constructed the URL and no --source/--host was provided in args, + // inject the source/host arg into the native command so the tool knows where to point. + if rc.repository != "" && sourceURL != "" && rubySourceFromArgs(rc.args) == "" { + rc.args = rubyInjectSourceArg(rc.nativeTool, subCommand, rc.args, sourceURL) + } + var extraEnv []string - if serverDetails != nil { + if serverDetails != nil && sourceURL != "" { extraEnv = rc.injectAuth(serverDetails, sourceURL) + // For gem install/fetch, embed credentials in the source URL so RubyGems + // uses them for index downloads (specs.4.8.gz). + if rc.nativeTool == toolGem && (subCommand == "install" || subCommand == "fetch") { + rc.args = rubyEmbedCredsInSourceArg(rc.args, serverDetails) + } + } else if serverDetails != nil && sourceURL == "" { + log.Debug("Ruby auth: no Artifactory gem source discovered in args/Gemfile/gem-sources — skipping credential injection") } log.Info(fmt.Sprintf("Running %s %s.", rc.nativeTool, subCommand)) @@ -84,6 +102,67 @@ func (rc *RubyCommand) Run() error { return nil } +// rubyEmbedCredsInSourceArg rewrites --source/--host URL args to embed credentials +// for gem install/fetch. RubyGems 3.x uses embedded URL credentials for index downloads +// (specs.4.8.gz) but does NOT use GEM_HOST_API_KEY for those requests. +func rubyEmbedCredsInSourceArg(args []string, serverDetails *coreConfig.ServerDetails) []string { + user, pass := rubyCredentials(serverDetails) + if user == "" || pass == "" { + return args + } + result := make([]string, len(args)) + copy(result, args) + for i, a := range result { + var rawURL string + var prefix string + switch { + case strings.HasPrefix(a, "--source="): + prefix = "--source=" + rawURL = strings.TrimPrefix(a, prefix) + case strings.HasPrefix(a, "--host="): + prefix = "--host=" + rawURL = strings.TrimPrefix(a, prefix) + case (a == "--source" || a == "-s" || a == "--host") && i+1 < len(result): + parsed, err := url.Parse(result[i+1]) + if err != nil || parsed.User != nil { + continue + } + parsed.User = url.UserPassword(user, pass) + result[i+1] = parsed.String() + continue + default: + continue + } + if rawURL == "" { + continue + } + parsed, err := url.Parse(rawURL) + if err != nil || parsed.User != nil { + continue + } + parsed.User = url.UserPassword(user, pass) + result[i] = prefix + parsed.String() + } + return result +} + +// rubyInjectSourceArg appends the appropriate source/host flag to native args when +// --repo was used to construct the URL and the user didn't provide one in their command. +// For gem push → --host; for gem install/fetch → --source; for bundle → no arg needed +// (Bundler uses env-var-based auth and reads from Gemfile, so the Gemfile must point +// at Artifactory — --repo only helps with credential injection for bundle). +func rubyInjectSourceArg(tool, subCommand string, args []string, sourceURL string) []string { + if tool == toolGem { + switch subCommand { + case "push": + return append(args, "--host", sourceURL) + case "install", "fetch": + return append(args, "--source", sourceURL) + } + } + return args +} + // runRubyBinary executes gem/bundle with stdio pass-through and optional extra env vars. func runRubyBinary(tool string, args, extraEnv []string) error { cmd := exec.Command(tool, args...) // #nosec G204 -- tool is restricted to gem/bundle; args come from the user's own command line @@ -98,7 +177,7 @@ func runRubyBinary(tool string, args, extraEnv []string) error { // isRubyHelpRequest reports whether the invocation is purely a help request. func isRubyHelpRequest(subCommand string, args []string) bool { - if subCommand == "help" || subCommand == "" { + if subCommand == "help" { return true } for _, a := range args { @@ -165,7 +244,8 @@ func (rc *RubyCommand) injectAuth(serverDetails *coreConfig.ServerDetails, sourc if os.Getenv("GEM_HOST_API_KEY") != "" { log.Info("Ruby auth [gem]: GEM_HOST_API_KEY already set — respecting existing credentials") } else { - extraEnv = append(extraEnv, fmt.Sprintf("GEM_HOST_API_KEY=%s:%s", user, pass)) + basicAuth := "Basic " + base64.StdEncoding.EncodeToString([]byte(user+":"+pass)) + extraEnv = append(extraEnv, fmt.Sprintf("GEM_HOST_API_KEY=%s", basicAuth)) log.Info("Ruby auth [gem]: injecting credentials via GEM_HOST_API_KEY") } } @@ -209,11 +289,23 @@ func bundleEnvKeyForHost(host string) string { // ── Repository discovery ─────────────────────────────────────────────────────── // resolveRepo discovers the Artifactory gem source URL and repo key the project uses. -// Precedence: explicit --repo override > --source/--host/--clear-sources arg > -// Gemfile `source` line > `gem sources` list. Returns empty strings when none is found. -func (rc *RubyCommand) resolveRepo(workingDir string) (sourceURL, repoKey string) { +// Precedence: explicit --repo override (URL constructed from server config) > +// --source/--host/--clear-sources arg > Gemfile `source` line > `gem sources` list. +// Returns empty strings when none is found. +func (rc *RubyCommand) resolveRepo(workingDir string, serverDetails *coreConfig.ServerDetails) (sourceURL, repoKey string) { + // When --repo is provided, construct the full Artifactory gems URL from server config. if rc.repository != "" { - return "", rc.repository + if serverDetails == nil { + log.Warn("Ruby: --repo specified but no server details available; using repo key only") + return "", rc.repository + } + repoURL, err := rubyConstructRepoURL(serverDetails, rc.repository) + if err != nil { + log.Warn(fmt.Sprintf("Ruby: failed to construct repo URL from server config: %v; using repo key only", err)) + return "", rc.repository + } + log.Info(fmt.Sprintf("Ruby: using --repo %q → %s", rc.repository, repoURL)) + return repoURL, rc.repository } // 1. Inspect the command args for an explicit source/host URL. if u := rubySourceFromArgs(rc.args); u != "" { @@ -230,6 +322,23 @@ func (rc *RubyCommand) resolveRepo(workingDir string) (sourceURL, repoKey string return "", "" } +// rubyConstructRepoURL builds the Artifactory gems API URL from server details and repo name. +// Example: serverURL "https://my.jfrog.io/artifactory/" + repo "gems-virtual" +// → "https://my.jfrog.io/artifactory/api/gems/gems-virtual/" +func rubyConstructRepoURL(serverDetails *coreConfig.ServerDetails, repoName string) (string, error) { + baseURL := serverDetails.GetArtifactoryUrl() + parsed, err := url.Parse(baseURL) + if err != nil { + return "", err + } + parsed = parsed.JoinPath("api/gems", repoName) + result := parsed.String() + if !strings.HasSuffix(result, "/") { + result += "/" + } + return result, nil +} + // rubySourceFromArgs returns the URL following --source/-s/--host/--clear-sources flags, // or an inline "--source=" form. func rubySourceFromArgs(args []string) string { @@ -653,7 +762,7 @@ func rubyEnrichDepsFromArtifactory(deps []buildinfo.Dependency, repoKey string, orClauses = append(orClauses, fmt.Sprintf(`{"name":{"$match":%q}}`, e.prefix+"*.gem")) } aqlQuery := fmt.Sprintf( - `items.find({"repo":%q,"$or":[%s]}).include("name","actual_sha1","actual_md5","sha256")`, + `items.find({"repo":%q,"$or":[%s]}).include("name","path","actual_sha1","actual_md5","sha256")`, searchRepo, strings.Join(orClauses, ","), ) @@ -668,6 +777,7 @@ func rubyEnrichDepsFromArtifactory(deps []buildinfo.Dependency, repoKey string, var aqlResult struct { Results []struct { Name string `json:"name"` + Path string `json:"path"` ActualSha1 string `json:"actual_sha1"` ActualMd5 string `json:"actual_md5"` Sha256 string `json:"sha256"` @@ -694,6 +804,12 @@ func rubyEnrichDepsFromArtifactory(deps []buildinfo.Dependency, repoKey string, if r.Sha256 != "" && deps[e.idx].Sha256 == "" { deps[e.idx].Sha256 = r.Sha256 } + // Set the repository path for the dependency (repo/path/filename). + if r.Path != "" && r.Path != "." { + deps[e.idx].Repository = searchRepo + "/" + r.Path + "/" + r.Name + } else { + deps[e.idx].Repository = searchRepo + "/" + r.Name + } enriched++ break } diff --git a/artifactory/commands/ruby/native_ruby_test.go b/artifactory/commands/ruby/native_ruby_test.go index 8894e624..3c83d057 100644 --- a/artifactory/commands/ruby/native_ruby_test.go +++ b/artifactory/commands/ruby/native_ruby_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "testing" + coreConfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/stretchr/testify/assert" ) @@ -69,7 +70,7 @@ gem "rails" func TestIsRubyHelpRequest(t *testing.T) { assert.True(t, isRubyHelpRequest("help", []string{"help"})) - assert.True(t, isRubyHelpRequest("", nil)) + assert.False(t, isRubyHelpRequest("", nil)) // empty subCommand is now caught before help check assert.True(t, isRubyHelpRequest("install", []string{"install", "--help"})) assert.True(t, isRubyHelpRequest("install", []string{"install", "-h"})) assert.False(t, isRubyHelpRequest("install", []string{"install", "rake"})) @@ -118,3 +119,95 @@ func TestRubyRunUnsupportedTool(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "unsupported ruby tool") } + +func TestRubyRunNoArgs(t *testing.T) { + cmd := NewRubyCommand().SetNativeTool("gem").SetArgs(nil) + err := cmd.Run() + assert.Error(t, err) + assert.Contains(t, err.Error(), "no subcommand provided") +} + +func TestRubyEmbedCredsInSourceArg(t *testing.T) { + server := &coreConfig.ServerDetails{ + User: "myuser", + Password: "mypass", + ArtifactoryUrl: "https://my.jfrog.io/artifactory/", + } + + // --source= form + args := []string{"install", "rake", "--source=https://my.jfrog.io/artifactory/api/gems/gems-virtual/"} + result := rubyEmbedCredsInSourceArg(args, server) + assert.Contains(t, result[2], "myuser:mypass@") + assert.Contains(t, result[2], "--source=https://myuser:mypass@") + + // --source form (separate arg) + args2 := []string{"install", "rake", "--source", "https://my.jfrog.io/artifactory/api/gems/gems-virtual/"} + result2 := rubyEmbedCredsInSourceArg(args2, server) + assert.Contains(t, result2[3], "myuser:mypass@") + + // -s short form + args3 := []string{"fetch", "rake", "-s", "https://my.jfrog.io/artifactory/api/gems/gems-virtual/"} + result3 := rubyEmbedCredsInSourceArg(args3, server) + assert.Contains(t, result3[3], "myuser:mypass@") + + // URL already has credentials — should not double-embed + args4 := []string{"install", "rake", "--source=https://other:creds@my.jfrog.io/api/gems/r/"} + result4 := rubyEmbedCredsInSourceArg(args4, server) + assert.Equal(t, args4[2], result4[2]) + + // No --source/--host — args unchanged + args5 := []string{"install", "rake"} + result5 := rubyEmbedCredsInSourceArg(args5, server) + assert.Equal(t, args5, result5) +} + +func TestRubyConstructRepoURL(t *testing.T) { + server := &coreConfig.ServerDetails{ + ArtifactoryUrl: "https://my.jfrog.io/artifactory/", + } + + u, err := rubyConstructRepoURL(server, "gems-virtual") + assert.NoError(t, err) + assert.Equal(t, "https://my.jfrog.io/artifactory/api/gems/gems-virtual/", u) + + u2, err := rubyConstructRepoURL(server, "gems-local") + assert.NoError(t, err) + assert.Equal(t, "https://my.jfrog.io/artifactory/api/gems/gems-local/", u2) + + // Without trailing slash on base URL + server2 := &coreConfig.ServerDetails{ + ArtifactoryUrl: "https://my.jfrog.io/artifactory", + } + u3, err := rubyConstructRepoURL(server2, "gems-remote") + assert.NoError(t, err) + assert.Equal(t, "https://my.jfrog.io/artifactory/api/gems/gems-remote/", u3) +} + +func TestRubyInjectSourceArg(t *testing.T) { + sourceURL := "https://my.jfrog.io/artifactory/api/gems/gems-virtual/" + + // gem push → --host + args := rubyInjectSourceArg(toolGem, "push", []string{"push", "my.gem"}, sourceURL) + assert.Contains(t, args, "--host") + assert.Contains(t, args, sourceURL) + + // gem install → --source + args2 := rubyInjectSourceArg(toolGem, "install", []string{"install", "rake"}, sourceURL) + assert.Contains(t, args2, "--source") + assert.Contains(t, args2, sourceURL) + + // gem fetch → --source + args3 := rubyInjectSourceArg(toolGem, "fetch", []string{"fetch", "rake"}, sourceURL) + assert.Contains(t, args3, "--source") + assert.Contains(t, args3, sourceURL) + + // gem build → no injection (doesn't need source) + args4 := rubyInjectSourceArg(toolGem, "build", []string{"build", "my.gemspec"}, sourceURL) + assert.NotContains(t, args4, "--source") + assert.NotContains(t, args4, "--host") + + // bundle install → no injection (bundle uses env var auth, not --source) + args5 := rubyInjectSourceArg(toolBundle, "install", []string{"install"}, sourceURL) + assert.NotContains(t, args5, "--source") + assert.NotContains(t, args5, "--host") +} From 57ca8587a590d435d9f356019baf0710942634f9 Mon Sep 17 00:00:00 2001 From: agrasth Date: Thu, 2 Jul 2026 14:54:11 +0530 Subject: [PATCH 03/24] feat: add scope classification and jf setup for Ruby Scope classification: - New parseGemfileGroups() parses Gemfile group blocks and inline groups - Classifies gems as production/development/test - Transitive deps inherit scopes from their parents - Scopes passed to build-info for Xray scanning jf setup integration: - Added project.Ruby to packageManagerToRepositoryPackageType (Gems) - New configureRuby() method configures Bundler credentials via 'bundle config set' and adds source to ~/.gemrc - Ruby now appears in 'jf setup' supported package managers list Co-authored-by: Cursor --- artifactory/commands/ruby/native_ruby.go | 141 ++++++++++++++++++ artifactory/commands/ruby/native_ruby_test.go | 53 +++++++ artifactory/commands/setup/setup.go | 75 ++++++++++ 3 files changed, 269 insertions(+) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index bef3e3ec..5d4ef9b1 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -503,6 +503,8 @@ func (rc *RubyCommand) collectDependencyBuildInfo(workingDir, subCommand, repoKe } gemConfig := flexpack.GemConfig{WorkingDirectory: workingDir} + // Parse Gemfile groups for scope classification (production/development/test). + gemConfig.GemGroups = parseGemfileGroups(workingDir) // For bundler, use `bundle list` as the ground-truth installed set so group // filtering (--without/--with) is reflected accurately. if rc.nativeTool == toolBundle { @@ -710,6 +712,145 @@ func parseBundleListLine(line string) (name, version string) { return name, version } +// parseGemfileGroups parses the Gemfile to extract gem → group mappings. +// Returns a map where keys are gem names and values are their Bundler groups. +// Gems outside any group block get ["production"]. Gems inside `group :dev do...end` +// get ["development"], etc. Gems in multiple groups get all of them. +func parseGemfileGroups(workingDir string) map[string][]string { + gemfilePath := filepath.Join(workingDir, "Gemfile") + data, err := os.ReadFile(gemfilePath) + if err != nil { + return nil + } + + groups := make(map[string][]string) + var currentGroups []string // nil = top level (production) + + scanner := bufio.NewScanner(strings.NewReader(string(data))) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + + // Skip comments and empty lines. + if line == "" || strings.HasPrefix(line, "#") { + continue + } + + // Detect `group :development do` or `group :development, :test do` + if strings.HasPrefix(line, "group") && strings.HasSuffix(line, "do") { + currentGroups = parseGroupNames(line) + continue + } + + // Detect `end` closing a group block. + if line == "end" && currentGroups != nil { + currentGroups = nil + continue + } + + // Detect `gem "name"` declarations. + gemName := parseGemDeclaration(line) + if gemName == "" { + continue + } + + // Inline group: `gem "rspec", group: :test` or `gem "rspec", groups: [:test, :development]` + if inlineGroups := parseInlineGroups(line); len(inlineGroups) > 0 { + groups[gemName] = inlineGroups + } else if currentGroups != nil { + groups[gemName] = currentGroups + } else { + groups[gemName] = []string{"production"} + } + } + + if len(groups) == 0 { + return nil + } + return groups +} + +// parseGroupNames extracts group names from `group :dev, :test do`. +func parseGroupNames(line string) []string { + // Strip "group " prefix and " do" suffix. + line = strings.TrimPrefix(line, "group") + line = strings.TrimSuffix(line, "do") + line = strings.TrimSpace(line) + + var result []string + for _, part := range strings.Split(line, ",") { + part = strings.TrimSpace(part) + part = strings.TrimPrefix(part, ":") + part = strings.Trim(part, `"'`) + if part != "" { + result = append(result, part) + } + } + return result +} + +// parseGemDeclaration extracts the gem name from `gem "name"` or `gem 'name'`. +func parseGemDeclaration(line string) string { + if !strings.HasPrefix(line, "gem ") && !strings.HasPrefix(line, "gem\t") { + return "" + } + rest := strings.TrimPrefix(line, "gem") + rest = strings.TrimSpace(rest) + // Extract quoted name. + if len(rest) < 3 { + return "" + } + quote := rest[0] + if quote != '"' && quote != '\'' { + return "" + } + endIdx := strings.IndexByte(rest[1:], quote) + if endIdx == -1 { + return "" + } + return rest[1 : endIdx+1] +} + +// parseInlineGroups handles `gem "x", group: :test` or `gem "x", groups: [:dev, :test]`. +func parseInlineGroups(line string) []string { + // Look for group: or groups: in the line. + idx := strings.Index(line, "group:") + if idx == -1 { + idx = strings.Index(line, "groups:") + if idx == -1 { + return nil + } + } + rest := line[idx:] + colonIdx := strings.IndexByte(rest, ':') + if colonIdx == -1 { + return nil + } + rest = strings.TrimSpace(rest[colonIdx+1:]) + + // Handle array form: [:dev, :test] + if strings.HasPrefix(rest, "[") { + rest = strings.TrimPrefix(rest, "[") + rest = strings.TrimSuffix(strings.TrimSpace(rest), "]") + // Remove trailing stuff after the bracket + if closeIdx := strings.IndexByte(rest, ']'); closeIdx != -1 { + rest = rest[:closeIdx] + } + } + + var result []string + for _, part := range strings.Split(rest, ",") { + part = strings.TrimSpace(part) + part = strings.TrimPrefix(part, ":") + part = strings.Trim(part, `"'`) + // Remove trailing non-alphanumeric (e.g., closing bracket remnants) + part = strings.TrimRight(part, " \t])") + if part != "" && !strings.Contains(part, " ") { + result = append(result, part) + } + } + return result +} + // rubyEnrichDepsFromArtifactory fetches sha1/sha256/md5 for registry-based dependencies // in a single batched AQL call, matching .gem filenames by "-" prefix. // GIT/PATH deps (in directURLDeps) are skipped since they are not stored in Artifactory. diff --git a/artifactory/commands/ruby/native_ruby_test.go b/artifactory/commands/ruby/native_ruby_test.go index 3c83d057..2529cc7b 100644 --- a/artifactory/commands/ruby/native_ruby_test.go +++ b/artifactory/commands/ruby/native_ruby_test.go @@ -211,3 +211,56 @@ func TestRubyInjectSourceArg(t *testing.T) { assert.NotContains(t, args5, "--source") assert.NotContains(t, args5, "--host") } + +func TestParseGemfileGroups(t *testing.T) { + dir := t.TempDir() + gemfile := `source "https://rubygems.org" + +gem "rails" +gem "puma" + +group :development do + gem "pry" + gem "rubocop" +end + +group :test do + gem "rspec" +end + +group :development, :test do + gem "faker" +end +` + if err := os.WriteFile(filepath.Join(dir, "Gemfile"), []byte(gemfile), 0644); err != nil { + t.Fatal(err) + } + + groups := parseGemfileGroups(dir) + assert.NotNil(t, groups) + + // Top-level gems → production + assert.Equal(t, []string{"production"}, groups["rails"]) + assert.Equal(t, []string{"production"}, groups["puma"]) + + // Development group + assert.Equal(t, []string{"development"}, groups["pry"]) + assert.Equal(t, []string{"development"}, groups["rubocop"]) + + // Test group + assert.Equal(t, []string{"test"}, groups["rspec"]) + + // Multi-group + assert.ElementsMatch(t, []string{"development", "test"}, groups["faker"]) + + // No Gemfile → nil + assert.Nil(t, parseGemfileGroups(t.TempDir())) +} + +func TestParseGemDeclaration(t *testing.T) { + assert.Equal(t, "rails", parseGemDeclaration(`gem "rails"`)) + assert.Equal(t, "rails", parseGemDeclaration(`gem 'rails'`)) + assert.Equal(t, "rails", parseGemDeclaration(`gem "rails", "~> 7.0"`)) + assert.Equal(t, "", parseGemDeclaration(`source "https://rubygems.org"`)) + assert.Equal(t, "", parseGemDeclaration(`# gem "commented"`)) +} diff --git a/artifactory/commands/setup/setup.go b/artifactory/commands/setup/setup.go index 85651aa6..71123674 100644 --- a/artifactory/commands/setup/setup.go +++ b/artifactory/commands/setup/setup.go @@ -18,6 +18,7 @@ import ( container "github.com/jfrog/jfrog-cli-artifactory/artifactory/commands/ocicontainer" "github.com/jfrog/jfrog-cli-artifactory/artifactory/commands/python" "github.com/jfrog/jfrog-cli-artifactory/artifactory/commands/repository" + "github.com/jfrog/jfrog-cli-artifactory/artifactory/commands/ruby" commandsutils "github.com/jfrog/jfrog-cli-core/v2/artifactory/commands/utils" "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils" "github.com/jfrog/jfrog-cli-core/v2/artifactory/utils/maven" @@ -61,6 +62,8 @@ var packageManagerToRepositoryPackageType = map[project.ProjectType]string{ project.Gradle: repository.Gradle, project.Maven: repository.Maven, + + project.Ruby: repository.Gems, } // SetupCommand configures registries and authentication for various package manager (npm, Yarn, Pip, Pipenv, Poetry, UV, Go) @@ -184,6 +187,8 @@ func (sc *SetupCommand) Run() (err error) { err = sc.configureMaven() case project.UV: err = sc.configureUV() + case project.Ruby: + err = sc.configureRuby() default: err = errorutils.CheckErrorf("unsupported package manager: %s", sc.packageManager) } @@ -570,6 +575,76 @@ func (sc *SetupCommand) configureUV() error { return nil } +// configureRuby configures RubyGems and Bundler to use Artifactory as a gem source. +// It performs: +// 1. `bundle config set :` (Bundler per-host credentials) +// 2. Adds the Artifactory source to `~/.gemrc` or prints guidance for Gemfile +// +// Both gem and bundle tools will then authenticate to the Artifactory gems repository. +func (sc *SetupCommand) configureRuby() error { + repoUrl, username, password, err := ruby.GetRubyGemsRepoUrlWithCredentials(sc.serverDetails, sc.repoName) + if err != nil { + return fmt.Errorf("failed to get RubyGems repository URL with credentials: %w", err) + } + + // If no credentials are provided, just print guidance. + if username == "" && password == "" { + log.Output(fmt.Sprintf("Add this source to your Gemfile:\n source \"%s\"\n", repoUrl.String())) + return nil + } + + host := repoUrl.Hostname() + if repoUrl.Port() != "" { + host += ":" + repoUrl.Port() + } + + // Configure Bundler: `bundle config set :` + bundleCmd := exec.Command("bundle", "config", "set", host, username+":"+password) + bundleCmd.Stdout = io.Discard + bundleCmd.Stderr = os.Stderr + if bundleErr := bundleCmd.Run(); bundleErr != nil { + log.Warn("Failed to configure Bundler credentials (bundle may not be installed): " + bundleErr.Error()) + } else { + log.Info(fmt.Sprintf("Bundler configured: credentials set for host '%s'", host)) + } + + // Configure gem: add source to ~/.gemrc if not already present. + sourceURL := repoUrl.String() + if gemrcErr := rubyAddSourceToGemrc(sourceURL); gemrcErr != nil { + log.Debug("Could not update ~/.gemrc: " + gemrcErr.Error()) + } + + log.Output(fmt.Sprintf("\nAdd this source to your Gemfile:\n source \"%s\"\n", sourceURL)) + return nil +} + +// rubyAddSourceToGemrc adds the Artifactory gems URL to ~/.gemrc :sources if not present. +func rubyAddSourceToGemrc(sourceURL string) error { + home, err := os.UserHomeDir() + if err != nil { + return err + } + gemrcPath := home + "/.gemrc" + + // Read existing content. + existing, _ := os.ReadFile(gemrcPath) + content := string(existing) + + // If the source is already there, skip. + if strings.Contains(content, sourceURL) { + return nil + } + + // Append a :sources entry. gemrc is YAML-like but simple enough to append. + if !strings.Contains(content, ":sources:") { + content += "\n:sources:\n- https://rubygems.org\n- " + sourceURL + "\n" + } else { + content += "- " + sourceURL + "\n" + } + + return os.WriteFile(gemrcPath, []byte(content), 0644) +} + // configureHelm configures Helm to use Artifactory as an OCI registry. // It executes: // From d694c9fa925f67246489a9635f91fec48c0fd964 Mon Sep 17 00:00:00 2001 From: agrasth Date: Fri, 10 Jul 2026 11:05:44 +0530 Subject: [PATCH 04/24] fix: hybrid checksums, remove gem build collection, gem push auth improvements - Remove gem build from artifact collection (local-only, no Artifactory path) - Only gem push now records artifacts in build-info - Implement hybrid checksum strategy: local gem cache first, AQL fallback - Add gem push auth via temporary ~/.gem/credentials (all RubyGems versions) - Embed credentials in --source URL for gem install/fetch index downloads - Strip trailing slash from --host to prevent 405 double-slash issue - Skip auth injection entirely for gem build (pure local operation) - Remove dead parseGemBuildOutputFile function and its tests Co-authored-by: Cursor --- artifactory/commands/ruby/native_ruby.go | 602 +++++++++++++++--- artifactory/commands/ruby/native_ruby_test.go | 334 +++++++++- 2 files changed, 854 insertions(+), 82 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index 5d4ef9b1..b929add5 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -75,26 +75,65 @@ func (rc *RubyCommand) Run() error { } var extraEnv []string - if serverDetails != nil && sourceURL != "" { + var credCleanup func() + // gem build is a pure local operation — skip auth injection entirely. + if rc.nativeTool == toolGem && subCommand == "build" { + log.Debug("Ruby auth: skipping credential injection for gem build (local-only operation)") + } else if serverDetails != nil && sourceURL != "" { extraEnv = rc.injectAuth(serverDetails, sourceURL) - // For gem install/fetch, embed credentials in the source URL so RubyGems - // uses them for index downloads (specs.4.8.gz). - if rc.nativeTool == toolGem && (subCommand == "install" || subCommand == "fetch") { - rc.args = rubyEmbedCredsInSourceArg(rc.args, serverDetails) + if rc.nativeTool == toolGem { + switch subCommand { + case "install", "fetch": + // Embed credentials in --source URL for index downloads (specs.4.8.gz). + rc.args = rubyEmbedCredsInSourceArg(rc.args, serverDetails) + log.Debug("Ruby auth [gem install/fetch]: embedded credentials in --source URL for index downloads") + case "push": + // Strip trailing slash from --host in args (whether user-provided or injected). + // RubyGems' push_command.rb builds URLs as "#{host}/api/v1/gems" — a trailing + // slash on host produces a double-slash that Artifactory rejects with 405. + rc.args = rubyStripHostTrailingSlash(rc.args) + // Write temporary ~/.gem/credentials for the target host. + // CRITICAL: the credentials key MUST exactly match the --host value that + // gets passed to the native command (no trailing slash). + pushHost := strings.TrimRight(sourceURL, "/") + cleanup, credErr := rubyWriteTempGemCredentials(pushHost, serverDetails) + if credErr != nil { + log.Warn("Ruby auth [gem push]: failed to write temporary credentials: " + credErr.Error()) + } else { + credCleanup = cleanup + log.Debug("Ruby auth [gem push]: wrote temporary ~/.gem/credentials entry for " + pushHost) + } + } } } else if serverDetails != nil && sourceURL == "" { log.Debug("Ruby auth: no Artifactory gem source discovered in args/Gemfile/gem-sources — skipping credential injection") } + defer func() { + if credCleanup != nil { + credCleanup() + } + }() log.Info(fmt.Sprintf("Running %s %s.", rc.nativeTool, subCommand)) - if runErr := runRubyBinary(rc.nativeTool, rc.args, extraEnv); runErr != nil { - return fmt.Errorf("%s %s failed: %w", rc.nativeTool, subCommand, runErr) + // For gem install/fetch, capture stdout to parse "Successfully installed"/"Downloaded" lines. + var capturedOutput string + needsCapture := rc.nativeTool == toolGem && (subCommand == "install" || subCommand == "fetch") + if needsCapture { + var runErr error + capturedOutput, runErr = runRubyBinaryCapture(rc.nativeTool, rc.args, extraEnv) + if runErr != nil { + return fmt.Errorf("%s %s failed: %w", rc.nativeTool, subCommand, runErr) + } + } else { + if runErr := runRubyBinary(rc.nativeTool, rc.args, extraEnv); runErr != nil { + return fmt.Errorf("%s %s failed: %w", rc.nativeTool, subCommand, runErr) + } } if rc.buildConfiguration != nil { buildName, nameErr := rc.buildConfiguration.GetBuildName() if nameErr == nil && buildName != "" { - if biErr := rc.collectBuildInfo(workingDir, subCommand, repoKey, serverDetails); biErr != nil { + if biErr := rc.collectBuildInfo(workingDir, subCommand, repoKey, serverDetails, capturedOutput); biErr != nil { log.Warn("Failed to collect Ruby build info: " + biErr.Error()) } } @@ -146,6 +185,130 @@ func rubyEmbedCredsInSourceArg(args []string, serverDetails *coreConfig.ServerDe return result } +// rubyEmbedCredsInHostArg embeds credentials in the --host URL for gem push. +// This is the fallback for RubyGems <= 3.0.x which does NOT respect GEM_HOST_API_KEY. +// Uses the same logic as rubyEmbedCredsInSourceArg but only targets --host. +func rubyEmbedCredsInHostArg(args []string, serverDetails *coreConfig.ServerDetails) []string { + user, pass := rubyCredentials(serverDetails) + if user == "" || pass == "" { + return args + } + result := make([]string, len(args)) + copy(result, args) + for i, a := range result { + var rawURL string + var prefix string + switch { + case strings.HasPrefix(a, "--host="): + prefix = "--host=" + rawURL = strings.TrimPrefix(a, prefix) + case a == "--host" && i+1 < len(result): + parsed, err := url.Parse(result[i+1]) + if err != nil || parsed.User != nil { + continue + } + parsed.User = url.UserPassword(user, pass) + result[i+1] = parsed.String() + continue + default: + continue + } + if rawURL == "" { + continue + } + parsed, err := url.Parse(rawURL) + if err != nil || parsed.User != nil { + continue + } + parsed.User = url.UserPassword(user, pass) + result[i] = prefix + parsed.String() + } + return result +} + +// rubyWriteTempGemCredentials writes a temporary entry to ~/.gem/credentials for gem push. +// This is the only auth mechanism that works across ALL RubyGems versions (3.0.x through current). +// RubyGems' push_command.rb ALWAYS checks ~/.gem/credentials keyed by host URL. +// Returns a cleanup function that restores the original file (or removes the added entry). +func rubyWriteTempGemCredentials(hostURL string, serverDetails *coreConfig.ServerDetails) (cleanup func(), err error) { + user, pass := rubyCredentials(serverDetails) + if user == "" || pass == "" { + return nil, fmt.Errorf("no credentials available") + } + + homeDir, err := os.UserHomeDir() + if err != nil { + return nil, fmt.Errorf("could not determine home directory: %w", err) + } + gemDir := filepath.Join(homeDir, ".gem") + credFile := filepath.Join(gemDir, "credentials") + + // Ensure ~/.gem directory exists. + if err := os.MkdirAll(gemDir, 0700); err != nil { + return nil, fmt.Errorf("could not create ~/.gem directory: %w", err) + } + + // Read existing credentials file (if any) to preserve and restore later. + var originalContent []byte + var originalExists bool + if data, readErr := os.ReadFile(credFile); readErr == nil { + originalContent = data + originalExists = true + } + + // The credential value: Basic auth encoded (what Artifactory expects). + credValue := "Basic " + base64.StdEncoding.EncodeToString([]byte(user+":"+pass)) + + // CRITICAL: Use the host URL EXACTLY as-is (including trailing slash if present). + // RubyGems' Gem::GemcutterUtilities#api_key does an exact string match against + // the --host value. If we strip the trailing slash but --host has one, the lookup misses. + credKey := hostURL + + // Build new credentials content: preserve existing + add our entry. + var newContent string + if originalExists { + newContent = string(originalContent) + // Remove existing entry for same host if present (we'll re-add it). + // Check both with and without trailing slash to avoid duplicates. + keyWithSlash := strings.TrimRight(hostURL, "/") + "/" + keyWithoutSlash := strings.TrimRight(hostURL, "/") + lines := strings.Split(newContent, "\n") + var filtered []string + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, keyWithSlash+":") || strings.HasPrefix(trimmed, keyWithoutSlash+":") { + continue + } + filtered = append(filtered, line) + } + newContent = strings.Join(filtered, "\n") + } else { + newContent = "---\n" + } + + // Add our entry keyed by the EXACT host URL (preserving trailing slash). + if !strings.HasSuffix(newContent, "\n") { + newContent += "\n" + } + newContent += fmt.Sprintf("%s: %s\n", credKey, credValue) + + // Write the credentials file with restricted permissions (0600 required by RubyGems). + if err := os.WriteFile(credFile, []byte(newContent), 0600); err != nil { + return nil, fmt.Errorf("could not write credentials file: %w", err) + } + + // Return cleanup function. + cleanup = func() { + if originalExists { + _ = os.WriteFile(credFile, originalContent, 0600) + } else { + _ = os.Remove(credFile) + } + log.Debug("Ruby auth [gem push]: cleaned up temporary ~/.gem/credentials entry") + } + return cleanup, nil +} + // rubyInjectSourceArg appends the appropriate source/host flag to native args when // --repo was used to construct the URL and the user didn't provide one in their command. // For gem push → --host; for gem install/fetch → --source; for bundle → no arg needed @@ -155,7 +318,11 @@ func rubyInjectSourceArg(tool, subCommand string, args []string, sourceURL strin if tool == toolGem { switch subCommand { case "push": - return append(args, "--host", sourceURL) + // Strip trailing slash for gem push: RubyGems' push_command.rb builds + // the request URL as "#{host}/api/v1/gems" — if host already ends with /, + // the resulting URL has a double slash which Artifactory rejects with 405. + hostForPush := strings.TrimRight(sourceURL, "/") + return append(args, "--host", hostForPush) case "install", "fetch": return append(args, "--source", sourceURL) } @@ -163,6 +330,24 @@ func rubyInjectSourceArg(tool, subCommand string, args []string, sourceURL strin return args } +// rubyStripHostTrailingSlash removes the trailing slash from any --host value in args. +// For gem push, RubyGems builds URLs as "#{host}/api/v1/gems" — a trailing slash on +// the host creates a double-slash that Artifactory rejects with 405. +func rubyStripHostTrailingSlash(args []string) []string { + result := make([]string, len(args)) + copy(result, args) + for i, a := range result { + switch { + case strings.HasPrefix(a, "--host="): + val := strings.TrimPrefix(a, "--host=") + result[i] = "--host=" + strings.TrimRight(val, "/") + case a == "--host" && i+1 < len(result): + result[i+1] = strings.TrimRight(result[i+1], "/") + } + } + return result +} + // runRubyBinary executes gem/bundle with stdio pass-through and optional extra env vars. func runRubyBinary(tool string, args, extraEnv []string) error { cmd := exec.Command(tool, args...) // #nosec G204 -- tool is restricted to gem/bundle; args come from the user's own command line @@ -175,6 +360,23 @@ func runRubyBinary(tool string, args, extraEnv []string) error { return cmd.Run() } +// runRubyBinaryCapture executes gem/bundle capturing stdout while still printing it. +// Used for `gem install`/`fetch` to parse installed/downloaded gem names from output. +func runRubyBinaryCapture(tool string, args, extraEnv []string) (string, error) { + cmd := exec.Command(tool, args...) // #nosec G204 + cmd.Stdin = os.Stdin + cmd.Stderr = os.Stderr + if len(extraEnv) > 0 { + cmd.Env = append(os.Environ(), extraEnv...) + } + // Capture stdout while also printing it to the user's terminal. + var buf strings.Builder + cmd.Stdout = io.MultiWriter(os.Stdout, &buf) + err := cmd.Run() + return buf.String(), err +} + + // isRubyHelpRequest reports whether the invocation is purely a help request. func isRubyHelpRequest(subCommand string, args []string) bool { if subCommand == "help" { @@ -246,7 +448,7 @@ func (rc *RubyCommand) injectAuth(serverDetails *coreConfig.ServerDetails, sourc } else { basicAuth := "Basic " + base64.StdEncoding.EncodeToString([]byte(user+":"+pass)) extraEnv = append(extraEnv, fmt.Sprintf("GEM_HOST_API_KEY=%s", basicAuth)) - log.Info("Ruby auth [gem]: injecting credentials via GEM_HOST_API_KEY") + log.Info("Ruby auth [gem]: injecting GEM_HOST_API_KEY (used by gem push on RubyGems >= 3.1; URL-embedded credentials used as primary auth for install/fetch/push)") } } return extraEnv @@ -458,12 +660,15 @@ func rubyHostMatchesServer(rawURL, artifactoryURL string) bool { // ── Build info ───────────────────────────────────────────────────────────────── // collectBuildInfo dispatches build-info collection based on the native tool/sub-command. -func (rc *RubyCommand) collectBuildInfo(workingDir, subCommand, repoKey string, serverDetails *coreConfig.ServerDetails) error { +// capturedOutput is the captured stdout from gem commands (empty for bundle commands). +func (rc *RubyCommand) collectBuildInfo(workingDir, subCommand, repoKey string, serverDetails *coreConfig.ServerDetails, capturedOutput string) error { switch { - case rc.nativeTool == toolGem && (subCommand == "build" || subCommand == "push"): - return rc.collectGemArtifactBuildInfo(workingDir, subCommand, repoKey, serverDetails) + case rc.nativeTool == toolGem && subCommand == "push": + // Only gem push records artifacts — it's the point where the .gem enters Artifactory. + // gem build is local-only; the artifact has no Artifactory path until pushed. + return rc.collectGemArtifactBuildInfo(workingDir, repoKey, serverDetails) case rc.collectsDependencies(subCommand): - return rc.collectDependencyBuildInfo(workingDir, subCommand, repoKey, serverDetails) + return rc.collectDependencyBuildInfo(workingDir, subCommand, repoKey, serverDetails, capturedOutput) default: log.Debug(fmt.Sprintf("Ruby build-info: no collection for '%s %s'", rc.nativeTool, subCommand)) return nil @@ -490,9 +695,10 @@ func (rc *RubyCommand) collectsDependencies(subCommand string) bool { return false } -// collectDependencyBuildInfo parses Gemfile.lock and records dependencies, enriching -// checksums from Artifactory. -func (rc *RubyCommand) collectDependencyBuildInfo(workingDir, subCommand, repoKey string, serverDetails *coreConfig.ServerDetails) error { +// collectDependencyBuildInfo records dependencies in build-info. For bundle commands, +// this parses Gemfile.lock via FlexPack. For gem install/fetch, it records the specific +// gems that were actually installed/fetched (parsed from the native tool's stdout). +func (rc *RubyCommand) collectDependencyBuildInfo(workingDir, subCommand, repoKey string, serverDetails *coreConfig.ServerDetails, capturedOutput string) error { buildName, err := rc.buildConfiguration.GetBuildName() if err != nil { return err @@ -502,14 +708,15 @@ func (rc *RubyCommand) collectDependencyBuildInfo(workingDir, subCommand, repoKe return err } + // For gem install/fetch: parse stdout to determine exactly what was installed/fetched. + if rc.nativeTool == toolGem { + return rc.collectGemInstallDependencies(workingDir, subCommand, buildName, buildNumber, repoKey, serverDetails, capturedOutput) + } + + // For bundle install/update/lock/add: use the full FlexPack lock-file parser. gemConfig := flexpack.GemConfig{WorkingDirectory: workingDir} - // Parse Gemfile groups for scope classification (production/development/test). gemConfig.GemGroups = parseGemfileGroups(workingDir) - // For bundler, use `bundle list` as the ground-truth installed set so group - // filtering (--without/--with) is reflected accurately. - if rc.nativeTool == toolBundle { - gemConfig.InstalledPackages = bundleInstalledPackages(workingDir) - } + gemConfig.InstalledPackages = bundleInstalledPackages(workingDir) collector, err := flexpack.NewRubygemsFlexPack(gemConfig) if err != nil { @@ -526,7 +733,7 @@ func (rc *RubyCommand) collectDependencyBuildInfo(workingDir, subCommand, repoKe if len(bi.Modules) > 0 && len(bi.Modules[0].Dependencies) > 0 && repoKey != "" && serverDetails != nil { directURLDeps := collector.GetDirectURLDeps() - rubyEnrichDepsFromArtifactory(bi.Modules[0].Dependencies, repoKey, directURLDeps, serverDetails) + rubyEnrichDepsChecksums(bi.Modules[0].Dependencies, repoKey, directURLDeps, serverDetails) } else if repoKey == "" { log.Info("Ruby build-info: no Artifactory gems repo discovered — dependency checksum enrichment skipped. " + "Point your Gemfile/gem source at an Artifactory gems repository or pass --server-id.") @@ -539,8 +746,202 @@ func (rc *RubyCommand) collectDependencyBuildInfo(workingDir, subCommand, repoKe return nil } -// collectGemArtifactBuildInfo records the .gem artifact produced by `gem build`/`gem push`. -func (rc *RubyCommand) collectGemArtifactBuildInfo(workingDir, subCommand, repoKey string, serverDetails *coreConfig.ServerDetails) error { +// collectGemInstallDependencies records the gems that were actually installed/fetched. +// Primary mechanism: parse stdout ("Successfully installed X-Y" / "Downloaded X-Y.gem"). +// Fallback: explicit -v/--version arg + gem name from args, or gem list query. +func (rc *RubyCommand) collectGemInstallDependencies(workingDir, subCommand, buildName, buildNumber, repoKey string, serverDetails *coreConfig.ServerDetails, capturedOutput string) error { + // Primary: parse the captured stdout for definitive name:version pairs. + deps := parseGemCommandOutput(capturedOutput, subCommand) + + // Fallback: if stdout parsing yielded nothing, try extracting from args + gem list. + if len(deps) == 0 { + explicitVersion := extractVersionFromArgs(rc.args) + gemNames := extractGemNamesFromArgs(rc.args) + for _, name := range gemNames { + version := explicitVersion + if version == "" { + version = queryInstalledGemVersion(name) + } + if version == "" { + log.Debug(fmt.Sprintf("Ruby build-info [gem %s]: could not determine version for %q — skipping", subCommand, name)) + continue + } + deps = append(deps, buildinfo.Dependency{ + Id: fmt.Sprintf("%s:%s", name, version), + Type: gemDepArtifactType, + }) + } + } + + if len(deps) == 0 { + log.Debug(fmt.Sprintf("Ruby build-info [gem %s]: no gems detected in output or args — empty build-info", subCommand)) + return nil + } + + moduleID := "gem-" + subCommand + if customModule := rc.buildConfiguration.GetModule(); customModule != "" { + moduleID = customModule + } + + bi := &buildinfo.BuildInfo{ + Name: buildName, + Number: buildNumber, + Agent: &buildinfo.Agent{Name: "gem"}, + BuildAgent: &buildinfo.Agent{Name: "Generic", Version: "1.0"}, + Modules: []buildinfo.Module{{ + Id: moduleID, + Type: buildinfo.Gem, + Dependencies: deps, + }}, + } + + // Enrich checksums from Artifactory. + if repoKey != "" && serverDetails != nil { + rubyEnrichDepsChecksums(bi.Modules[0].Dependencies, repoKey, nil, serverDetails) + } + + if err := rubySaveBuildInfo(bi, rc.buildConfiguration); err != nil { + return fmt.Errorf("failed to save RubyGems build info: %w", err) + } + log.Info(fmt.Sprintf("RubyGems build info collected (%d gem(s)). Use 'jf rt bp %s %s' to publish.", len(deps), buildName, buildNumber)) + return nil +} + +// extractGemNamesFromArgs parses gem names from `gem install/fetch` command args. +// Skips flags (--source, --version, etc.) and their values. +func extractGemNamesFromArgs(args []string) []string { + var names []string + skipNext := false + for i, a := range args { + if i == 0 { + continue // skip the subcommand itself (install/fetch) + } + if skipNext { + skipNext = false + continue + } + // Skip flags and their values. + if strings.HasPrefix(a, "-") { + // Flags that take a value argument. + switch a { + case "--source", "-s", "--host", "--version", "-v", "--platform", "-i", + "--install-dir", "--bindir", "-n", "--document", "--build-root": + skipNext = true + } + continue + } + // Skip anything that looks like a path or URL (not a gem name). + if strings.Contains(a, "/") || strings.Contains(a, "\\") { + continue + } + names = append(names, a) + } + return names +} + +// parseGemCommandOutput parses gem install/fetch stdout to extract name:version pairs. +// +// gem install prints: "Successfully installed -" +// gem fetch prints: "Downloaded -.gem" or "Fetching: -.gem" +// +// This is the primary (most accurate) mechanism — it reflects what actually happened. +func parseGemCommandOutput(output, subCommand string) []buildinfo.Dependency { + if output == "" { + return nil + } + seen := make(map[string]bool) + var deps []buildinfo.Dependency + scanner := bufio.NewScanner(strings.NewReader(output)) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + var nameVersion string + switch { + case strings.HasPrefix(line, "Successfully installed "): + // "Successfully installed colorize-1.1.0" + nameVersion = strings.TrimPrefix(line, "Successfully installed ") + case strings.HasPrefix(line, "Downloaded "): + // "Downloaded colorize-1.1.0.gem" + nameVersion = strings.TrimSuffix(strings.TrimPrefix(line, "Downloaded "), ".gem") + case strings.HasPrefix(line, "Fetching: "): + // "Fetching: colorize-1.1.0.gem (100%)" — older gem versions + nameVersion = strings.TrimPrefix(line, "Fetching: ") + if idx := strings.Index(nameVersion, ".gem"); idx > 0 { + nameVersion = nameVersion[:idx] + } + default: + continue + } + name, version := splitGemNameVersion(nameVersion) + if name == "" || version == "" { + continue + } + depID := fmt.Sprintf("%s:%s", name, version) + if seen[depID] { + continue + } + seen[depID] = true + deps = append(deps, buildinfo.Dependency{ + Id: depID, + Type: gemDepArtifactType, + }) + } + return deps +} + +// splitGemNameVersion splits "colorize-1.1.0" into ("colorize", "1.1.0"). +// Gem names can contain hyphens (e.g., "rspec-core"), so we split on the LAST +// hyphen that is followed by a digit. +func splitGemNameVersion(s string) (name, version string) { + for i := len(s) - 1; i >= 0; i-- { + if s[i] == '-' && i+1 < len(s) && s[i+1] >= '0' && s[i+1] <= '9' { + return s[:i], s[i+1:] + } + } + return "", "" +} + +// extractVersionFromArgs extracts an explicit version from -v/--version flags in args. +func extractVersionFromArgs(args []string) string { + for i, a := range args { + switch { + case (a == "-v" || a == "--version") && i+1 < len(args): + return strings.TrimSpace(args[i+1]) + case strings.HasPrefix(a, "--version="): + return strings.TrimSpace(strings.TrimPrefix(a, "--version=")) + case strings.HasPrefix(a, "-v") && len(a) > 2 && a[2] != '-': + // -v1.0.0 form (unusual but valid) + return strings.TrimSpace(a[2:]) + } + } + return "" +} + +// queryInstalledGemVersion queries the installed version of a gem via `gem list --exact `. +// Used as a fallback when stdout parsing doesn't yield results. +// Returns the latest installed version or empty string if not found. +func queryInstalledGemVersion(name string) string { + cmd := exec.Command("gem", "list", "--exact", name) + out, err := cmd.Output() + if err != nil { + return "" + } + // Output format: "colorize (1.1.0, 0.8.1)" or "colorize (1.1.0)" + line := strings.TrimSpace(string(out)) + openParen := strings.Index(line, "(") + closeParen := strings.Index(line, ")") + if openParen == -1 || closeParen == -1 || closeParen <= openParen { + return "" + } + versions := line[openParen+1 : closeParen] + parts := strings.Split(versions, ",") + if len(parts) == 0 { + return "" + } + return strings.TrimSpace(parts[0]) +} + +// collectGemArtifactBuildInfo records the .gem artifact uploaded by `gem push`. +func (rc *RubyCommand) collectGemArtifactBuildInfo(workingDir, repoKey string, serverDetails *coreConfig.ServerDetails) error { buildName, err := rc.buildConfiguration.GetBuildName() if err != nil { return err @@ -576,8 +977,7 @@ func (rc *RubyCommand) collectGemArtifactBuildInfo(workingDir, subCommand, repoK }}, } - // On push, set build properties on the uploaded artifacts in Artifactory. - if subCommand == "push" && repoKey != "" && serverDetails != nil { + if repoKey != "" && serverDetails != nil { if propErr := rubySetBuildProperties(serverDetails, repoKey, buildName, buildNumber, rc.buildConfiguration.GetProject(), bi); propErr != nil { log.Warn("Failed to set build properties on gem artifacts: " + propErr.Error()) } @@ -599,37 +999,21 @@ func (rc *RubyCommand) gemModuleID(workingDir string) string { return name } -// rubyCollectGemArtifacts locates .gem files (build output or explicit push target) -// and computes their checksums for build-info. +// rubyCollectGemArtifacts locates the .gem file from the `gem push` command args. func rubyCollectGemArtifacts(workingDir string, args []string) ([]buildinfo.Artifact, error) { - gemFiles := make(map[string]bool) - - // Explicit .gem path on a `gem push ` command. + var gemFiles []string for _, a := range args { - if strings.HasSuffix(a, ".gem") { + if strings.HasSuffix(a, ".gem") && !strings.HasPrefix(a, "-") { p := a if !filepath.IsAbs(p) { p = filepath.Join(workingDir, a) } - gemFiles[p] = true - } - } - - // `gem build` writes -.gem into the working dir and pkg/. - for _, dir := range []string{workingDir, filepath.Join(workingDir, "pkg")} { - entries, err := os.ReadDir(dir) - if err != nil { - continue - } - for _, e := range entries { - if !e.IsDir() && strings.HasSuffix(e.Name(), ".gem") { - gemFiles[filepath.Join(dir, e.Name())] = true - } + gemFiles = append(gemFiles, p) } } var artifacts []buildinfo.Artifact - for path := range gemFiles { + for _, path := range gemFiles { checksum, err := rubyFileChecksums(path) if err != nil { log.Warn(fmt.Sprintf("Could not compute checksums for %s: %v", path, err)) @@ -851,29 +1235,22 @@ func parseInlineGroups(line string) []string { return result } -// rubyEnrichDepsFromArtifactory fetches sha1/sha256/md5 for registry-based dependencies -// in a single batched AQL call, matching .gem filenames by "-" prefix. +// rubyDepEntry associates a dependency index with its gem filename prefix for enrichment. +type rubyDepEntry struct { + idx int + prefix string // "-" used to match the .gem filename +} + +// rubyEnrichDepsChecksums enriches dependency checksums using a hybrid approach: +// 1. Try local gem cache first (fast, no network) +// 2. Fall back to AQL for any that couldn't be resolved locally // GIT/PATH deps (in directURLDeps) are skipped since they are not stored in Artifactory. -func rubyEnrichDepsFromArtifactory(deps []buildinfo.Dependency, repoKey string, directURLDeps map[string]string, serverDetails *coreConfig.ServerDetails) { +func rubyEnrichDepsChecksums(deps []buildinfo.Dependency, repoKey string, directURLDeps map[string]string, serverDetails *coreConfig.ServerDetails) { if len(deps) == 0 { return } - servicesManager, err := utils.CreateServiceManager(serverDetails, -1, 0, false) - if err != nil { - log.Warn("Could not create services manager for dependency enrichment: " + err.Error()) - return - } - searchRepo, err := utils.GetRepoNameForDependenciesSearch(repoKey, servicesManager) - if err != nil { - log.Warn("Could not resolve repo for dependency search, using as-is: " + err.Error()) - searchRepo = repoKey - } - type depEntry struct { - idx int - prefix string // "-" used to match the .gem filename - } - var entries []depEntry + var entries []rubyDepEntry for i, dep := range deps { if dep.Id == "" { continue @@ -886,12 +1263,80 @@ func rubyEnrichDepsFromArtifactory(deps []buildinfo.Dependency, repoKey string, continue } name, version := dep.Id[:colonIdx], dep.Id[colonIdx+1:] - entries = append(entries, depEntry{i, name + "-" + version}) + entries = append(entries, rubyDepEntry{i, name + "-" + version}) } if len(entries) == 0 { return } + // Phase 1: Try local gem cache. + cacheDir := rubyGemCacheDir() + localHits := 0 + var needsAQL []rubyDepEntry + for _, e := range entries { + if cacheDir == "" { + needsAQL = append(needsAQL, e) + continue + } + gemFile := filepath.Join(cacheDir, e.prefix+".gem") + checksum, err := rubyFileChecksums(gemFile) + if err == nil { + deps[e.idx].Sha1 = checksum.Sha1 + deps[e.idx].Md5 = checksum.Md5 + deps[e.idx].Sha256 = checksum.Sha256 + localHits++ + log.Debug(fmt.Sprintf("Checksum from local cache: %s", e.prefix)) + } else { + needsAQL = append(needsAQL, e) + } + } + if localHits > 0 { + log.Info(fmt.Sprintf("Resolved %d/%d dependency checksums from local gem cache", localHits, len(entries))) + } + + // Phase 2: AQL fallback for remaining deps (also provides repo path). + if len(needsAQL) == 0 && repoKey == "" { + return + } + // Even locally-resolved deps need repo path from AQL if we have a repo key. + entriesToQuery := needsAQL + if repoKey != "" { + entriesToQuery = entries + } + if len(entriesToQuery) == 0 || serverDetails == nil { + return + } + rubyEnrichDepsViaAQL(deps, entriesToQuery, repoKey, serverDetails) +} + +// rubyGemCacheDir returns the local RubyGems cache directory. +// Typically ~/.local/share/gem/ruby//cache or from `gem env gemdir`/cache. +func rubyGemCacheDir() string { + out, err := exec.Command("gem", "env", "gemdir").Output() + if err != nil { + log.Debug("Could not determine gem cache dir: " + err.Error()) + return "" + } + dir := filepath.Join(strings.TrimSpace(string(out)), "cache") + if info, statErr := os.Stat(dir); statErr == nil && info.IsDir() { + return dir + } + return "" +} + +// rubyEnrichDepsViaAQL fetches checksums and repo paths from Artifactory via batched AQL. +func rubyEnrichDepsViaAQL(deps []buildinfo.Dependency, entries []rubyDepEntry, repoKey string, serverDetails *coreConfig.ServerDetails) { + servicesManager, err := utils.CreateServiceManager(serverDetails, -1, 0, false) + if err != nil { + log.Warn("Could not create services manager for dependency enrichment: " + err.Error()) + return + } + searchRepo, err := utils.GetRepoNameForDependenciesSearch(repoKey, servicesManager) + if err != nil { + log.Warn("Could not resolve repo for dependency search, using as-is: " + err.Error()) + searchRepo = repoKey + } + var orClauses []string seen := make(map[string]bool) for _, e := range entries { @@ -899,13 +1344,13 @@ func rubyEnrichDepsFromArtifactory(deps []buildinfo.Dependency, repoKey string, continue } seen[e.prefix] = true - // Match "-.gem" and platform-specific "--.gem". orClauses = append(orClauses, fmt.Sprintf(`{"name":{"$match":%q}}`, e.prefix+"*.gem")) } aqlQuery := fmt.Sprintf( `items.find({"repo":%q,"$or":[%s]}).include("name","path","actual_sha1","actual_md5","sha256")`, searchRepo, strings.Join(orClauses, ","), ) + log.Debug(fmt.Sprintf("AQL fallback query for %d deps (repo: %s)", len(entries), searchRepo)) stream, err := servicesManager.Aql(aqlQuery) if err != nil { @@ -935,17 +1380,14 @@ func rubyEnrichDepsFromArtifactory(deps []buildinfo.Dependency, repoKey string, continue } for _, e := range entries { - if deps[e.idx].Sha1 != "" { - continue - } - // "-.gem" or "--.gem" if r.Name == e.prefix+".gem" || strings.HasPrefix(r.Name, e.prefix+"-") { - deps[e.idx].Sha1 = r.ActualSha1 - deps[e.idx].Md5 = r.ActualMd5 - if r.Sha256 != "" && deps[e.idx].Sha256 == "" { - deps[e.idx].Sha256 = r.Sha256 + if deps[e.idx].Sha1 == "" { + deps[e.idx].Sha1 = r.ActualSha1 + deps[e.idx].Md5 = r.ActualMd5 + if r.Sha256 != "" && deps[e.idx].Sha256 == "" { + deps[e.idx].Sha256 = r.Sha256 + } } - // Set the repository path for the dependency (repo/path/filename). if r.Path != "" && r.Path != "." { deps[e.idx].Repository = searchRepo + "/" + r.Path + "/" + r.Name } else { @@ -958,9 +1400,9 @@ func rubyEnrichDepsFromArtifactory(deps []buildinfo.Dependency, repoKey string, } if enriched > 0 { - log.Info(fmt.Sprintf("Enriched %d/%d RubyGems dependencies with Artifactory checksums (repo: %s)", enriched, len(deps), searchRepo)) + log.Info(fmt.Sprintf("Enriched %d/%d dependencies via AQL (repo: %s)", enriched, len(entries), searchRepo)) } else { - log.Debug(fmt.Sprintf("No RubyGems dependencies enriched from repo %s — gems may not be cached yet", searchRepo)) + log.Debug(fmt.Sprintf("No dependencies enriched via AQL from repo %s — gems may not be cached yet", searchRepo)) } } diff --git a/artifactory/commands/ruby/native_ruby_test.go b/artifactory/commands/ruby/native_ruby_test.go index 2529cc7b..b3e690e8 100644 --- a/artifactory/commands/ruby/native_ruby_test.go +++ b/artifactory/commands/ruby/native_ruby_test.go @@ -186,10 +186,11 @@ func TestRubyConstructRepoURL(t *testing.T) { func TestRubyInjectSourceArg(t *testing.T) { sourceURL := "https://my.jfrog.io/artifactory/api/gems/gems-virtual/" - // gem push → --host + // gem push → --host (trailing slash stripped to avoid double-slash in push URL) args := rubyInjectSourceArg(toolGem, "push", []string{"push", "my.gem"}, sourceURL) assert.Contains(t, args, "--host") - assert.Contains(t, args, sourceURL) + assert.Contains(t, args, "https://my.jfrog.io/artifactory/api/gems/gems-virtual") + assert.NotContains(t, args, sourceURL) // trailing slash must be gone // gem install → --source args2 := rubyInjectSourceArg(toolGem, "install", []string{"install", "rake"}, sourceURL) @@ -210,6 +211,28 @@ func TestRubyInjectSourceArg(t *testing.T) { args5 := rubyInjectSourceArg(toolBundle, "install", []string{"install"}, sourceURL) assert.NotContains(t, args5, "--source") assert.NotContains(t, args5, "--host") + + // gem push with URL that has no trailing slash — should pass through unchanged + args6 := rubyInjectSourceArg(toolGem, "push", []string{"push", "my.gem"}, "https://host/api/gems/repo") + assert.Contains(t, args6, "https://host/api/gems/repo") +} + +func TestRubyStripHostTrailingSlash(t *testing.T) { + // --host form + args := rubyStripHostTrailingSlash([]string{"push", "my.gem", "--host", "https://host/api/gems/repo/"}) + assert.Equal(t, "https://host/api/gems/repo", args[3]) + + // --host= form + args2 := rubyStripHostTrailingSlash([]string{"push", "my.gem", "--host=https://host/api/gems/repo/"}) + assert.Equal(t, "--host=https://host/api/gems/repo", args2[2]) + + // No trailing slash — unchanged + args3 := rubyStripHostTrailingSlash([]string{"push", "my.gem", "--host", "https://host/api/gems/repo"}) + assert.Equal(t, "https://host/api/gems/repo", args3[3]) + + // No --host at all — unchanged + args4 := rubyStripHostTrailingSlash([]string{"push", "my.gem"}) + assert.Equal(t, []string{"push", "my.gem"}, args4) } func TestParseGemfileGroups(t *testing.T) { @@ -264,3 +287,310 @@ func TestParseGemDeclaration(t *testing.T) { assert.Equal(t, "", parseGemDeclaration(`source "https://rubygems.org"`)) assert.Equal(t, "", parseGemDeclaration(`# gem "commented"`)) } + +func TestExtractGemNamesFromArgs(t *testing.T) { + // Simple install + names := extractGemNamesFromArgs([]string{"install", "colorize"}) + assert.Equal(t, []string{"colorize"}, names) + + // Multiple gems + names = extractGemNamesFromArgs([]string{"install", "colorize", "rake", "puma"}) + assert.Equal(t, []string{"colorize", "rake", "puma"}, names) + + // With --source flag (skip flag and value) + names = extractGemNamesFromArgs([]string{"install", "colorize", "--source", "https://my.jfrog.io/api/gems/r/"}) + assert.Equal(t, []string{"colorize"}, names) + + // With --version flag + names = extractGemNamesFromArgs([]string{"install", "colorize", "--version", "1.0.0"}) + assert.Equal(t, []string{"colorize"}, names) + + // With -v flag (short) + names = extractGemNamesFromArgs([]string{"install", "rails", "-v", "7.0.0"}) + assert.Equal(t, []string{"rails"}, names) + + // Fetch subcommand + names = extractGemNamesFromArgs([]string{"fetch", "rake"}) + assert.Equal(t, []string{"rake"}, names) + + // Boolean flags (no value) + names = extractGemNamesFromArgs([]string{"install", "rake", "--no-document", "--conservative"}) + assert.Equal(t, []string{"rake"}, names) + + // Path-like arg skipped + names = extractGemNamesFromArgs([]string{"install", "/path/to/some.gem"}) + assert.Empty(t, names) + + // No gem names (only flags) + names = extractGemNamesFromArgs([]string{"install", "--source", "https://example.com"}) + assert.Empty(t, names) +} + + +func TestRubyEmbedCredsInHostArg(t *testing.T) { + server := &coreConfig.ServerDetails{ + User: "myuser", + Password: "mypass", + ArtifactoryUrl: "https://my.jfrog.io/artifactory/", + } + + // --host= form + args := []string{"push", "my.gem", "--host=https://my.jfrog.io/artifactory/api/gems/gems-local/"} + result := rubyEmbedCredsInHostArg(args, server) + assert.Contains(t, result[2], "myuser:mypass@") + assert.Contains(t, result[2], "--host=https://myuser:mypass@") + + // --host form (separate arg) + args2 := []string{"push", "my.gem", "--host", "https://my.jfrog.io/artifactory/api/gems/gems-local/"} + result2 := rubyEmbedCredsInHostArg(args2, server) + assert.Contains(t, result2[3], "myuser:mypass@") + + // URL already has credentials — no double-embed + args3 := []string{"push", "my.gem", "--host=https://other:creds@my.jfrog.io/api/gems/r/"} + result3 := rubyEmbedCredsInHostArg(args3, server) + assert.Equal(t, args3[2], result3[2]) + + // --source flag (not --host) — should NOT be modified + args4 := []string{"install", "rake", "--source=https://my.jfrog.io/artifactory/api/gems/r/"} + result4 := rubyEmbedCredsInHostArg(args4, server) + assert.Equal(t, args4[2], result4[2]) + + // No --host — args unchanged + args5 := []string{"push", "my.gem"} + result5 := rubyEmbedCredsInHostArg(args5, server) + assert.Equal(t, args5, result5) +} + +func TestCollectsDependencies(t *testing.T) { + cmd := NewRubyCommand() + + cmd.nativeTool = toolBundle + assert.True(t, cmd.collectsDependencies("install")) + assert.True(t, cmd.collectsDependencies("update")) + assert.True(t, cmd.collectsDependencies("lock")) + assert.True(t, cmd.collectsDependencies("add")) + assert.False(t, cmd.collectsDependencies("exec")) + + cmd.nativeTool = toolGem + assert.True(t, cmd.collectsDependencies("install")) + assert.True(t, cmd.collectsDependencies("fetch")) + assert.False(t, cmd.collectsDependencies("build")) + assert.False(t, cmd.collectsDependencies("push")) +} + +func TestParseGemCommandOutput_Install(t *testing.T) { + // Standard gem install output with transitive deps + output := `Fetching: activesupport-7.0.4.gem (100%) +Successfully installed activesupport-7.0.4 +Fetching: actionpack-7.0.4.gem (100%) +Successfully installed actionpack-7.0.4 +Fetching: railties-7.0.4.gem (100%) +Successfully installed railties-7.0.4 +Successfully installed rails-7.0.4 +4 gems installed +` + deps := parseGemCommandOutput(output, "install") + assert.Len(t, deps, 4) + assert.Equal(t, "activesupport:7.0.4", deps[0].Id) + assert.Equal(t, "actionpack:7.0.4", deps[1].Id) + assert.Equal(t, "railties:7.0.4", deps[2].Id) + assert.Equal(t, "rails:7.0.4", deps[3].Id) +} + +func TestParseGemCommandOutput_InstallSingle(t *testing.T) { + output := "Successfully installed colorize-1.1.0\n1 gem installed\n" + deps := parseGemCommandOutput(output, "install") + assert.Len(t, deps, 1) + assert.Equal(t, "colorize:1.1.0", deps[0].Id) +} + +func TestParseGemCommandOutput_InstallVersionPin(t *testing.T) { + // When installing an older version explicitly + output := "Successfully installed rake-13.0.1\n1 gem installed\n" + deps := parseGemCommandOutput(output, "install") + assert.Len(t, deps, 1) + assert.Equal(t, "rake:13.0.1", deps[0].Id) +} + +func TestParseGemCommandOutput_Fetch(t *testing.T) { + // gem fetch output + output := "Downloaded httparty-0.21.0.gem\n" + deps := parseGemCommandOutput(output, "fetch") + assert.Len(t, deps, 1) + assert.Equal(t, "httparty:0.21.0", deps[0].Id) +} + +func TestParseGemCommandOutput_FetchMultiple(t *testing.T) { + output := "Downloaded colorize-1.1.0.gem\nDownloaded rake-13.4.2.gem\n" + deps := parseGemCommandOutput(output, "fetch") + assert.Len(t, deps, 2) + assert.Equal(t, "colorize:1.1.0", deps[0].Id) + assert.Equal(t, "rake:13.4.2", deps[1].Id) +} + +func TestParseGemCommandOutput_FetchOlderFormat(t *testing.T) { + // Older RubyGems fetch format + output := "Fetching: rspec-core-3.12.0.gem (100%)\n" + deps := parseGemCommandOutput(output, "fetch") + assert.Len(t, deps, 1) + assert.Equal(t, "rspec-core:3.12.0", deps[0].Id) +} + +func TestParseGemCommandOutput_HyphenatedGemName(t *testing.T) { + // Gem name with hyphens (e.g., rspec-core, net-http) + output := "Successfully installed rspec-core-3.12.0\nSuccessfully installed net-http-0.4.1\n" + deps := parseGemCommandOutput(output, "install") + assert.Len(t, deps, 2) + assert.Equal(t, "rspec-core:3.12.0", deps[0].Id) + assert.Equal(t, "net-http:0.4.1", deps[1].Id) +} + +func TestParseGemCommandOutput_Empty(t *testing.T) { + deps := parseGemCommandOutput("", "install") + assert.Nil(t, deps) +} + +func TestParseGemCommandOutput_NoDeps(t *testing.T) { + // Output with no install/download lines (e.g., already installed) + output := "Successfully installed colorize-1.1.0\nBut this line has no prefix\n" + deps := parseGemCommandOutput(output, "install") + assert.Len(t, deps, 1) +} + +func TestParseGemCommandOutput_Deduplication(t *testing.T) { + // Same gem mentioned twice (should deduplicate) + output := "Successfully installed rake-13.4.2\nSuccessfully installed rake-13.4.2\n" + deps := parseGemCommandOutput(output, "install") + assert.Len(t, deps, 1) +} + +func TestSplitGemNameVersion(t *testing.T) { + cases := []struct { + input string + wantName string + wantVersion string + }{ + {"colorize-1.1.0", "colorize", "1.1.0"}, + {"rspec-core-3.12.0", "rspec-core", "3.12.0"}, + {"net-http-0.4.1", "net-http", "0.4.1"}, + {"rails-7.0.4.2", "rails", "7.0.4.2"}, + {"nokogiri-1.13.9-x86_64-linux", "nokogiri", "1.13.9-x86_64-linux"}, // platform suffix in version + {"", "", ""}, + {"noversion", "", ""}, + {"rake-13.4.2", "rake", "13.4.2"}, + } + for _, c := range cases { + name, version := splitGemNameVersion(c.input) + assert.Equal(t, c.wantName, name, "input %q name", c.input) + assert.Equal(t, c.wantVersion, version, "input %q version", c.input) + } +} + +func TestExtractVersionFromArgs(t *testing.T) { + // -v flag + assert.Equal(t, "13.0.1", extractVersionFromArgs([]string{"install", "rake", "-v", "13.0.1"})) + + // --version flag + assert.Equal(t, "7.0.0", extractVersionFromArgs([]string{"install", "rails", "--version", "7.0.0"})) + + // --version= form + assert.Equal(t, "1.2.3", extractVersionFromArgs([]string{"install", "gem", "--version=1.2.3"})) + + // No version flag + assert.Equal(t, "", extractVersionFromArgs([]string{"install", "rake"})) + + // -v without space (unusual but valid) + assert.Equal(t, "1.0.0", extractVersionFromArgs([]string{"install", "gem", "-v1.0.0"})) +} + +func TestRubyWriteTempGemCredentials(t *testing.T) { + // Use a temporary home directory to avoid touching real ~/.gem/credentials + origHome := os.Getenv("HOME") + tmpHome := t.TempDir() + os.Setenv("HOME", tmpHome) + defer os.Setenv("HOME", origHome) + + server := &coreConfig.ServerDetails{ + User: "admin", + Password: "secret", + ArtifactoryUrl: "https://my.jfrog.io/artifactory/", + } + + // Host URL WITH trailing slash — must be preserved exactly in the credentials key. + hostURL := "https://my.jfrog.io/artifactory/api/gems/gems-local/" + cleanup, err := rubyWriteTempGemCredentials(hostURL, server) + assert.NoError(t, err) + assert.NotNil(t, cleanup) + + // Verify credentials file was written with EXACT host URL (trailing slash preserved). + credFile := filepath.Join(tmpHome, ".gem", "credentials") + content, readErr := os.ReadFile(credFile) + assert.NoError(t, readErr) + // Key MUST include the trailing slash — RubyGems does exact string match against --host value. + assert.Contains(t, string(content), "https://my.jfrog.io/artifactory/api/gems/gems-local/: Basic ") + + // Run cleanup + cleanup() + + // File should be removed (didn't exist before) + _, statErr := os.Stat(credFile) + assert.True(t, os.IsNotExist(statErr)) +} + +func TestRubyWriteTempGemCredentials_TrailingSlashPreserved(t *testing.T) { + origHome := os.Getenv("HOME") + tmpHome := t.TempDir() + os.Setenv("HOME", tmpHome) + defer os.Setenv("HOME", origHome) + + server := &coreConfig.ServerDetails{ + User: "admin", + Password: "pass", + } + + // With trailing slash + cleanup1, err := rubyWriteTempGemCredentials("https://host/api/gems/repo/", server) + assert.NoError(t, err) + content, _ := os.ReadFile(filepath.Join(tmpHome, ".gem", "credentials")) + assert.Contains(t, string(content), "https://host/api/gems/repo/: Basic ") + cleanup1() + + // Without trailing slash + cleanup2, err := rubyWriteTempGemCredentials("https://host/api/gems/repo", server) + assert.NoError(t, err) + content, _ = os.ReadFile(filepath.Join(tmpHome, ".gem", "credentials")) + assert.Contains(t, string(content), "https://host/api/gems/repo: Basic ") + assert.NotContains(t, string(content), "https://host/api/gems/repo/: Basic ") + cleanup2() +} + +func TestRubyWriteTempGemCredentials_PreservesExisting(t *testing.T) { + origHome := os.Getenv("HOME") + tmpHome := t.TempDir() + os.Setenv("HOME", tmpHome) + defer os.Setenv("HOME", origHome) + + // Create pre-existing credentials + gemDir := filepath.Join(tmpHome, ".gem") + os.MkdirAll(gemDir, 0700) + existingContent := "---\n:rubygems_api_key: existing-key\n" + os.WriteFile(filepath.Join(gemDir, "credentials"), []byte(existingContent), 0600) + + server := &coreConfig.ServerDetails{ + User: "admin", + Password: "token123", + } + + cleanup, err := rubyWriteTempGemCredentials("https://host/api/gems/repo/", server) + assert.NoError(t, err) + + // File should have both entries (key preserves trailing slash) + content, _ := os.ReadFile(filepath.Join(gemDir, "credentials")) + assert.Contains(t, string(content), ":rubygems_api_key: existing-key") + assert.Contains(t, string(content), "https://host/api/gems/repo/: Basic ") + + // Cleanup should restore original + cleanup() + restored, _ := os.ReadFile(filepath.Join(gemDir, "credentials")) + assert.Equal(t, existingContent, string(restored)) +} From ffc6d1172035afcd488c55f3ea07a14bbb6f3e97 Mon Sep 17 00:00:00 2001 From: agrasth Date: Mon, 13 Jul 2026 10:51:15 +0530 Subject: [PATCH 05/24] fix: remove bundle lock from build-info collection bundle lock only resolves the dependency graph and writes Gemfile.lock without downloading any gems. No actual consumption from Artifactory happens, so recording dependencies in build-info is incorrect. Co-authored-by: Cursor --- artifactory/commands/ruby/native_ruby.go | 7 ++++--- artifactory/commands/ruby/native_ruby_test.go | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index b929add5..6cf63de0 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -675,12 +675,13 @@ func (rc *RubyCommand) collectBuildInfo(workingDir, subCommand, repoKey string, } } -// collectsDependencies reports whether the sub-command resolves a dependency tree -// (i.e. produces/uses a Gemfile.lock we can read). +// collectsDependencies reports whether the sub-command actually downloads/installs +// gems from a remote source. Only commands that consume gems should record build-info. +// `bundle lock` is excluded — it only resolves and writes Gemfile.lock without downloading. func (rc *RubyCommand) collectsDependencies(subCommand string) bool { if rc.nativeTool == toolBundle { switch subCommand { - case "install", "update", "lock", "add": + case "install", "update", "add": return true } } diff --git a/artifactory/commands/ruby/native_ruby_test.go b/artifactory/commands/ruby/native_ruby_test.go index b3e690e8..07b8425b 100644 --- a/artifactory/commands/ruby/native_ruby_test.go +++ b/artifactory/commands/ruby/native_ruby_test.go @@ -367,7 +367,7 @@ func TestCollectsDependencies(t *testing.T) { cmd.nativeTool = toolBundle assert.True(t, cmd.collectsDependencies("install")) assert.True(t, cmd.collectsDependencies("update")) - assert.True(t, cmd.collectsDependencies("lock")) + assert.False(t, cmd.collectsDependencies("lock")) assert.True(t, cmd.collectsDependencies("add")) assert.False(t, cmd.collectsDependencies("exec")) From 85f31d8e1a0d140d815b230a795335527a353cc8 Mon Sep 17 00:00:00 2001 From: agrasth Date: Thu, 16 Jul 2026 01:24:33 +0530 Subject: [PATCH 06/24] chore: update build-info-go replace to remote commit for CI Points to feature branch commit instead of local path so CI can resolve the dependency without a local checkout. Co-authored-by: Cursor --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 91dd8acf..9f407b1e 100644 --- a/go.mod +++ b/go.mod @@ -198,7 +198,7 @@ require ( sigs.k8s.io/yaml v1.6.0 // indirect ) -replace github.com/jfrog/build-info-go => ../build-info-go +replace github.com/jfrog/build-info-go => github.com/jfrog/build-info-go v1.13.1-0.20260715194847-6e04c9b133c8 // replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260604085947-7c110b77b4b4 diff --git a/go.sum b/go.sum index f4d88098..5d977cb3 100644 --- a/go.sum +++ b/go.sum @@ -378,6 +378,8 @@ github.com/jellydator/ttlcache/v3 v3.4.0 h1:YS4P125qQS0tNhtL6aeYkheEaB/m8HCqdMMP github.com/jellydator/ttlcache/v3 v3.4.0/go.mod h1:Hw9EgjymziQD3yGsQdf1FqFdpp7YjFMd4Srg5EJlgD4= github.com/jfrog/archiver/v3 v3.6.3 h1:hkAmPjBw393tPmQ07JknLNWFNZjXdy2xFEnOW9wwOxI= github.com/jfrog/archiver/v3 v3.6.3/go.mod h1:5V9l+Fte30Y4qe9dUOAd3yNTf8lmtVNuhKNrvI8PMhg= +github.com/jfrog/build-info-go v1.13.1-0.20260715194847-6e04c9b133c8 h1:rXEAstQ879wh+o99c1RBz34wYOy4RywwasaPtP5JkTs= +github.com/jfrog/build-info-go v1.13.1-0.20260715194847-6e04c9b133c8/go.mod h1:CYRUCvLKfyARjoJXLWAxce1qNUxTEtbRKAARkV42vpE= github.com/jfrog/froggit-go v1.21.1 h1:I/XUOO6GQ1d/rmBlM361F8T654C3ohIWrpw23xNL9JY= github.com/jfrog/froggit-go v1.21.1/go.mod h1:umBiakJB0CSPFfe0AHVaC3n9xsmUT7NGkDCny3bRchI= github.com/jfrog/gofrog v1.7.6 h1:QmfAiRzVyaI7JYGsB7cxfAJePAZTzFz0gRWZSE27c6s= From fb694e87dc53b106953718cce8078943c3a2848a Mon Sep 17 00:00:00 2001 From: agrasth Date: Thu, 16 Jul 2026 02:56:18 +0530 Subject: [PATCH 07/24] fix: allow auth with reference tokens (empty username) Artifactory accepts basic auth with an empty username when the password is a valid access token (reference or JWT). This matches the Go module proxy pattern. Changed credential check from requiring both user+pass to only requiring pass (token). Co-authored-by: Cursor --- artifactory/commands/ruby/native_ruby.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index 6cf63de0..2d883097 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -146,7 +146,7 @@ func (rc *RubyCommand) Run() error { // (specs.4.8.gz) but does NOT use GEM_HOST_API_KEY for those requests. func rubyEmbedCredsInSourceArg(args []string, serverDetails *coreConfig.ServerDetails) []string { user, pass := rubyCredentials(serverDetails) - if user == "" || pass == "" { + if !rubyHasCredentials(user, pass) { return args } result := make([]string, len(args)) @@ -190,7 +190,7 @@ func rubyEmbedCredsInSourceArg(args []string, serverDetails *coreConfig.ServerDe // Uses the same logic as rubyEmbedCredsInSourceArg but only targets --host. func rubyEmbedCredsInHostArg(args []string, serverDetails *coreConfig.ServerDetails) []string { user, pass := rubyCredentials(serverDetails) - if user == "" || pass == "" { + if !rubyHasCredentials(user, pass) { return args } result := make([]string, len(args)) @@ -232,7 +232,7 @@ func rubyEmbedCredsInHostArg(args []string, serverDetails *coreConfig.ServerDeta // Returns a cleanup function that restores the original file (or removes the added entry). func rubyWriteTempGemCredentials(hostURL string, serverDetails *coreConfig.ServerDetails) (cleanup func(), err error) { user, pass := rubyCredentials(serverDetails) - if user == "" || pass == "" { + if !rubyHasCredentials(user, pass) { return nil, fmt.Errorf("no credentials available") } @@ -410,7 +410,7 @@ func rubyResolveServerDetails(serverID string) (*coreConfig.ServerDetails, error // RubyGems → GEM_HOST_API_KEY="user:password" (used by `gem push`/`gem fetch`). func (rc *RubyCommand) injectAuth(serverDetails *coreConfig.ServerDetails, sourceURL string) []string { user, pass := rubyCredentials(serverDetails) - if user == "" || pass == "" { + if !rubyHasCredentials(user, pass) { log.Debug("Ruby auth: no username/password/token available in server config; relying on native configuration") return nil } @@ -464,9 +464,17 @@ func rubyCredentials(serverDetails *coreConfig.ServerDetails) (user, pass string } pass = serverDetails.GetAccessToken() } + // Artifactory accepts basic auth with an empty username when the password + // is a valid access token (reference or JWT). This matches the Go module + // proxy pattern which also uses url.UserPassword("", token). return user, pass } +// rubyHasCredentials returns true when at least a password or token is available. +func rubyHasCredentials(user, pass string) bool { + return pass != "" +} + // bundleEnvKeyForHost converts a host into Bundler's per-host credential env var name, // following Bundler's key normalization: uppercase, "." → "__", "-" → "___", and any // remaining non-alphanumeric character → "_", prefixed with "BUNDLE_". From f04a79d1bbc91c5b8c6cd218b4bd27dbee57e23c Mon Sep 17 00:00:00 2001 From: agrasth Date: Thu, 16 Jul 2026 03:16:46 +0530 Subject: [PATCH 08/24] fix: error when explicit --server-id is not found Previously when --server-id pointed to a non-existent config entry, the command logged a warning and fell through to rubygems.org. Now it returns a clear error, matching user intent. Co-authored-by: Cursor --- artifactory/commands/ruby/native_ruby.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index 2d883097..e9f018e7 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -61,6 +61,9 @@ func (rc *RubyCommand) Run() error { serverDetails, srvErr := rc.ServerDetails() if srvErr != nil { + if rc.serverID != "" { + return fmt.Errorf("server ID %q not found: %w", rc.serverID, srvErr) + } log.Warn("Ruby auth: could not load jf server config — " + srvErr.Error()) serverDetails = nil } From f58023de2dd2b8bb8946630fa87d3d5a7dd32c45 Mon Sep 17 00:00:00 2001 From: agrasth Date: Thu, 16 Jul 2026 03:26:59 +0530 Subject: [PATCH 09/24] fix: inject bundle credentials for hostname without port Bundler credential lookup may check only the hostname key (e.g., BUNDLE_LOCALHOST) ignoring the port. Now inject credentials under both the host:port key and hostname-only key to cover all versions. Co-authored-by: Cursor --- artifactory/commands/ruby/native_ruby.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index e9f018e7..f1a2a7cb 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -438,13 +438,24 @@ func (rc *RubyCommand) injectAuth(serverDetails *coreConfig.ServerDetails, sourc var extraEnv []string switch rc.nativeTool { case toolBundle: + cred := fmt.Sprintf("%s:%s", user, pass) key := bundleEnvKeyForHost(host) if os.Getenv(key) != "" { log.Info(fmt.Sprintf("Ruby auth [bundle]: %s already set — respecting existing credentials", key)) } else { - extraEnv = append(extraEnv, fmt.Sprintf("%s=%s:%s", key, user, pass)) + extraEnv = append(extraEnv, key+"="+cred) log.Info(fmt.Sprintf("Ruby auth [bundle]: injecting credentials via %s", key)) } + // Bundler's credential lookup may strip the port from the key (e.g., for + // localhost:8081 it may check BUNDLE_LOCALHOST rather than BUNDLE_LOCALHOST_8081). + // Inject credentials under the hostname-only key as well to cover all versions. + hostOnly := strings.Split(host, ":")[0] + if hostOnly != host { + keyNoPort := bundleEnvKeyForHost(hostOnly) + if os.Getenv(keyNoPort) == "" { + extraEnv = append(extraEnv, keyNoPort+"="+cred) + } + } case toolGem: if os.Getenv("GEM_HOST_API_KEY") != "" { log.Info("Ruby auth [gem]: GEM_HOST_API_KEY already set — respecting existing credentials") From fbf21e7aa0b44de748087444ca5a40e8f3c5f010 Mon Sep 17 00:00:00 2001 From: agrasth Date: Tue, 21 Jul 2026 23:48:02 +0530 Subject: [PATCH 10/24] fix: remove unverified local gem cache checksum lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hybrid checksum enrichment tried the local RubyGems cache before Artifactory, letting a file merely present in the cache stand in for what Artifactory actually served with no cross-check. Reproduced two ways: a gem version that only ever existed in the local cache, and a deliberately tampered cache file shadowing a correctly-installed real gem — both cases produced build-info checksums that never came from the configured repo, undermining provenance for Xray scanning, SBOM generation, and audit trails. Checksums now come exclusively from AQL-verified Artifactory lookups. --- artifactory/commands/ruby/native_ruby.go | 72 ++++--------------- artifactory/commands/ruby/native_ruby_test.go | 19 +++++ 2 files changed, 33 insertions(+), 58 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index f1a2a7cb..79340c1d 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -1264,12 +1264,20 @@ type rubyDepEntry struct { prefix string // "-" used to match the .gem filename } -// rubyEnrichDepsChecksums enriches dependency checksums using a hybrid approach: -// 1. Try local gem cache first (fast, no network) -// 2. Fall back to AQL for any that couldn't be resolved locally -// GIT/PATH deps (in directURLDeps) are skipped since they are not stored in Artifactory. +// rubyEnrichDepsChecksums enriches dependency checksums exclusively from Artifactory via AQL. +// +// A local-gem-cache-first fast path was tried here previously and has been removed: it let a +// file merely *present* in the local RubyGems cache stand in as proof of what Artifactory +// actually served, with no cross-check. That's a provenance bug, not an optimization — a stale, +// mismatched, or tampered file with the right filename would silently produce build-info +// checksums for a dependency that never came from the configured repo at all (confirmed via +// two reproductions: a version that only ever existed in the local cache, and a deliberately +// tampered cache file shadowing a correctly-installed real gem). Build-info checksums are relied +// on for provenance (Xray scanning, SBOM, audit trails), so they must always come from a source +// that actually verifies against Artifactory. GIT/PATH deps (in directURLDeps) are skipped since +// they are not stored in Artifactory. func rubyEnrichDepsChecksums(deps []buildinfo.Dependency, repoKey string, directURLDeps map[string]string, serverDetails *coreConfig.ServerDetails) { - if len(deps) == 0 { + if len(deps) == 0 || serverDetails == nil { return } @@ -1292,59 +1300,7 @@ func rubyEnrichDepsChecksums(deps []buildinfo.Dependency, repoKey string, direct return } - // Phase 1: Try local gem cache. - cacheDir := rubyGemCacheDir() - localHits := 0 - var needsAQL []rubyDepEntry - for _, e := range entries { - if cacheDir == "" { - needsAQL = append(needsAQL, e) - continue - } - gemFile := filepath.Join(cacheDir, e.prefix+".gem") - checksum, err := rubyFileChecksums(gemFile) - if err == nil { - deps[e.idx].Sha1 = checksum.Sha1 - deps[e.idx].Md5 = checksum.Md5 - deps[e.idx].Sha256 = checksum.Sha256 - localHits++ - log.Debug(fmt.Sprintf("Checksum from local cache: %s", e.prefix)) - } else { - needsAQL = append(needsAQL, e) - } - } - if localHits > 0 { - log.Info(fmt.Sprintf("Resolved %d/%d dependency checksums from local gem cache", localHits, len(entries))) - } - - // Phase 2: AQL fallback for remaining deps (also provides repo path). - if len(needsAQL) == 0 && repoKey == "" { - return - } - // Even locally-resolved deps need repo path from AQL if we have a repo key. - entriesToQuery := needsAQL - if repoKey != "" { - entriesToQuery = entries - } - if len(entriesToQuery) == 0 || serverDetails == nil { - return - } - rubyEnrichDepsViaAQL(deps, entriesToQuery, repoKey, serverDetails) -} - -// rubyGemCacheDir returns the local RubyGems cache directory. -// Typically ~/.local/share/gem/ruby//cache or from `gem env gemdir`/cache. -func rubyGemCacheDir() string { - out, err := exec.Command("gem", "env", "gemdir").Output() - if err != nil { - log.Debug("Could not determine gem cache dir: " + err.Error()) - return "" - } - dir := filepath.Join(strings.TrimSpace(string(out)), "cache") - if info, statErr := os.Stat(dir); statErr == nil && info.IsDir() { - return dir - } - return "" + rubyEnrichDepsViaAQL(deps, entries, repoKey, serverDetails) } // rubyEnrichDepsViaAQL fetches checksums and repo paths from Artifactory via batched AQL. diff --git a/artifactory/commands/ruby/native_ruby_test.go b/artifactory/commands/ruby/native_ruby_test.go index 07b8425b..60f27643 100644 --- a/artifactory/commands/ruby/native_ruby_test.go +++ b/artifactory/commands/ruby/native_ruby_test.go @@ -5,6 +5,7 @@ import ( "path/filepath" "testing" + buildinfo "github.com/jfrog/build-info-go/entities" coreConfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/stretchr/testify/assert" ) @@ -594,3 +595,21 @@ func TestRubyWriteTempGemCredentials_PreservesExisting(t *testing.T) { restored, _ := os.ReadFile(filepath.Join(gemDir, "credentials")) assert.Equal(t, existingContent, string(restored)) } + +// TestRubyEnrichDepsChecksums_NeverTrustsLocalCache guards against reintroducing a provenance +// bug: checksum enrichment must only ever come from a verified Artifactory (AQL) lookup, never +// from a file merely present in the local RubyGems cache. Without network access (serverDetails +// nil), enrichment must leave checksums empty rather than falling back to any local filesystem +// read — there must be no code path that can populate build-info checksums without going through +// Artifactory. +func TestRubyEnrichDepsChecksums_NeverTrustsLocalCache(t *testing.T) { + deps := []buildinfo.Dependency{ + {Id: "rake:13.4.2", Type: "gem"}, + } + + rubyEnrichDepsChecksums(deps, "my-gems-repo", nil, nil) + + assert.Empty(t, deps[0].Sha1, "checksum must not be populated without a verified Artifactory lookup") + assert.Empty(t, deps[0].Sha256, "checksum must not be populated without a verified Artifactory lookup") + assert.Empty(t, deps[0].Md5, "checksum must not be populated without a verified Artifactory lookup") +} From 8f17e90390f19e9a8d421d4170089f2222d78b2a Mon Sep 17 00:00:00 2001 From: agrasth Date: Fri, 31 Jul 2026 03:21:48 +0530 Subject: [PATCH 11/24] docs: add design spec for jf setup ruby credential/gemrc write fix Fixes two confirmed bugs: bundle config set is incompatible with Bundler 1.x (silently writes a garbage key while reporting success), and gemrc source writes use fragile string concatenation with no real dedup across different repos. Both get replaced with direct YAML read-modify-write, mirroring the cargo setup implementation's TOML approach. --- .../specs/2026-07-31-ruby-setup-fix-design.md | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-31-ruby-setup-fix-design.md diff --git a/docs/superpowers/specs/2026-07-31-ruby-setup-fix-design.md b/docs/superpowers/specs/2026-07-31-ruby-setup-fix-design.md new file mode 100644 index 00000000..e1ff965c --- /dev/null +++ b/docs/superpowers/specs/2026-07-31-ruby-setup-fix-design.md @@ -0,0 +1,64 @@ +# `jf setup ruby` — Fix Design + +**Date:** 2026-07-31 +**Scope:** Ruby-only. Does not touch cargo/alpine/apt/nuget setup implementations. + +## Problem + +`configureRuby()` (`artifactory/commands/setup/setup.go`) has two confirmed bugs, found via manual testing against a live Artifactory instance: + +1. **Broken credential write.** It shells out to `bundle config set `. Bundler ≥ 2.0 supports the `set` subcommand; Bundler 1.x (still the default on stock macOS system Ruby, and any Ruby install that predates 2.0's bundled Bundler) does not — its CLI is `bundle config NAME [VALUE]`, no subcommand. On 1.x, the command doesn't error; it silently misparses `set` as the config key name and the rest as a single value, writing a garbage `BUNDLE_SET` entry to `~/.bundle/config`. `jf setup ruby` reports success regardless (`"Bundler configured: credentials set for host '...'"`), so there is no signal anything went wrong. Even when the subprocess itself fails, the current code only logs a warning (`"Failed to configure Bundler credentials (bundle may not be installed)"`) and continues — a misdiagnosis, since the real cause is unrelated to whether `bundle` is installed. + +2. **Fragile `~/.gemrc` write.** `rubyAddSourceToGemrc()` uses raw string concatenation with a `strings.Contains(content, sourceURL)` substring check to avoid duplicates. This only recognizes a byte-identical repeat of the same URL. Configuring a *different* repo on a later run doesn't get deduplicated against the first — it just appends. Reproduced live: running `jf setup ruby` twice against two different repos on the same Artifactory host left both source lines in `~/.gemrc`, with no way to tell which was most recently configured. + +## Non-goals + +- Auto-editing the project's `Gemfile`. Bundler has no global source-redirect mechanism (unlike Cargo's `[source.crates-io] replace-with`), so the Gemfile edit remains an unavoidable manual step. Keeping this manual is also consistent with the "never write Gemfile" principle already in effect elsewhere in this feature (build-info collection, dependency discovery). +- Any change to cargo, alpine, apt, or nuget setup commands. +- A `--remove`/cleanup command (APT's setup has one; ruby's doesn't, and isn't gaining one here). Worth a future ask, not bundled into this fix. +- Bundler version detection/branching on the CLI syntax. Rejected in favor of not shelling out to `bundle config` at all (see below) — this avoids the whole class of "does this CLI syntax exist on this version" problem permanently, including against *future* Bundler CLI changes, not just the current 1.x/2.x split. + +## Design + +Both bugs are fixed the same way: stop shelling out to native CLIs for config writes, and read-modify-write the actual YAML files directly, the way `cargo/setup.go`'s `ConfigureNativeRegistry` already does for TOML. `gopkg.in/yaml.v3` is already a direct dependency of `jfrog-cli-core` (which `jfrog-cli-artifactory` already depends on), so this adds no new external dependency. + +### `writeBundleConfig(host, user, password string) error` + +Replaces the `exec.Command("bundle", "config", "set", ...)` call in `configureRuby()`. + +1. Read `~/.bundle/config`. Missing file → treat as empty. File exists but fails to parse as YAML → return an error (do not silently overwrite a file that may be hand-edited and load-bearing — matches Cargo's `mergeTomlFile` behavior: `err != nil && !os.IsNotExist(err) → return err`). +2. Compute the config key via the **existing** `bundleEnvKeyForHost(host)` function (already used by `jf ruby bundle install`'s runtime auth injection in `native_ruby.go`). Reusing it — rather than reimplementing host normalization — guarantees setup-time and runtime-injection key formats can never drift apart. +3. Set/overwrite that key to `"user:password"` in the parsed map. All other existing keys (`BUNDLE_PATH`, other hosts' credentials, anything else already in the file) are preserved untouched. +4. Marshal back to YAML and write, with file mode `0600` (this file now holds a real credential — the current code doesn't set this at all). +5. A write failure at any step is returned as a real error, which `configureRuby()` propagates up as a command failure (non-zero exit). This is a deliberate change from today's behavior: silently continuing after a failed credential write is the exact misdiagnosis this fix exists to eliminate. `jf setup ruby` should never report success when the credential wasn't actually written. + +### `addGemrcSource(sourceURL string) error` + +Replaces `rubyAddSourceToGemrc()`. + +1. Read `~/.gemrc`. Missing file → treat as empty. Parse failure on an existing file → error, same reasoning as above. +2. Preserve all unrelated top-level keys (e.g. `:ssl_ca_cert:`). +3. `:sources:` is a YAML list of strings. + - If the list doesn't exist yet, create it seeded with `https://rubygems.org` (matches current behavior). + - If `sourceURL` is already present (exact match) → no duplicate insert; move it to the front of the list (excluding `rubygems.org`, which stays first). + - If `sourceURL` is not present → prepend it (front of the list, after `rubygems.org`). + - Rationale for "append, don't replace, when different": `~/.gemrc`'s sources list is natively a multi-source mechanism — bare `gem install` already searches every listed source. Treating a second, different repo as something to *replace* the first with would fight that native behavior. Moving the most-recent one to the front makes it the one `gem` naturally tries first, and the one reflected in the printed "add this source to your Gemfile" suggestion. +4. Marshal back to YAML and write. + +### Error handling summary + +| Situation | Current behavior | New behavior | +|---|---|---| +| Bundler is 1.x | Silently writes garbage key, reports success | Writes a correct key directly; no dependency on Bundler CLI syntax at all | +| `~/.bundle/config` write fails | Logged as warning, command continues, reports success | Real error surfaced | +| `~/.gemrc` write fails | Logged at debug level only (`log.Debug`), essentially invisible | Real error surfaced | +| Existing file has unrelated keys | Preserved (string concat happens to not clobber them) | Preserved (explicit, via parse-modify-write) | +| Existing file is malformed/hand-edited | Silently appended to (string concat doesn't care about validity) | Clear error, no data loss risk | +| Re-run with the same repo | Skipped (works today) | Skipped, and moved to front | +| Re-run with a different repo | Appended without dedup guarantee, no ordering signal | Appended (intentional — see rationale above), moved to front | + +## Testing + +- Unit tests for `writeBundleConfig`, mirroring the existing `TestRubyWriteTempGemCredentials*` pattern (from the earlier `gem push` credentials fix in this same file): empty file, file with unrelated existing keys (preserved), overwrite of an existing same-host key, malformed existing file (errors, doesn't clobber). +- Unit tests for `addGemrcSource`: empty file, file with unrelated keys, append of a new source, re-add of an identical source (no duplicate, moved to front), append of a second different source (both present, most recent first). +- Manual/integration verification: the exact repro from manual testing — run `jf setup ruby` twice against two different repos on the same Artifactory host. Confirm `~/.gemrc` has both, most-recent first; confirm `~/.bundle/config` has exactly one correct entry for the shared host, using the same key `jf ruby bundle install` would inject at runtime. From 8a00d942ebdcda9adcfa8860f6951552e9e91a33 Mon Sep 17 00:00:00 2001 From: agrasth Date: Fri, 31 Jul 2026 03:30:30 +0530 Subject: [PATCH 12/24] fix: write Bundler/gemrc config directly instead of shelling out in jf setup ruby `bundle config set ` silently misparses on Bundler 1.x (no `set` subcommand there), writing a garbage BUNDLE_SET key while reporting success. ~/.gemrc writes also relied on fragile substring matching that couldn't dedupe across different repos. configureRuby now reads/writes ~/.bundle/config and ~/.gemrc as YAML directly (mirroring cargo/setup.go's TOML approach), reusing ruby.BundleEnvKeyForHost (now exported) for host-key normalization so setup-time and runtime-injection keys can't drift apart. A write failure is now a real error instead of a silently-ignored warning. Re-configuring a different repo keeps both sources in ~/.gemrc, most recent first, since gem install natively searches every listed source. --- artifactory/commands/ruby/native_ruby.go | 8 +- artifactory/commands/ruby/native_ruby_test.go | 2 +- artifactory/commands/setup/setup.go | 127 +++++++++++---- artifactory/commands/setup/setup_test.go | 146 ++++++++++++++++++ go.mod | 2 +- 5 files changed, 253 insertions(+), 32 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index 79340c1d..083e17ca 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -439,7 +439,7 @@ func (rc *RubyCommand) injectAuth(serverDetails *coreConfig.ServerDetails, sourc switch rc.nativeTool { case toolBundle: cred := fmt.Sprintf("%s:%s", user, pass) - key := bundleEnvKeyForHost(host) + key := BundleEnvKeyForHost(host) if os.Getenv(key) != "" { log.Info(fmt.Sprintf("Ruby auth [bundle]: %s already set — respecting existing credentials", key)) } else { @@ -451,7 +451,7 @@ func (rc *RubyCommand) injectAuth(serverDetails *coreConfig.ServerDetails, sourc // Inject credentials under the hostname-only key as well to cover all versions. hostOnly := strings.Split(host, ":")[0] if hostOnly != host { - keyNoPort := bundleEnvKeyForHost(hostOnly) + keyNoPort := BundleEnvKeyForHost(hostOnly) if os.Getenv(keyNoPort) == "" { extraEnv = append(extraEnv, keyNoPort+"="+cred) } @@ -489,12 +489,12 @@ func rubyHasCredentials(user, pass string) bool { return pass != "" } -// bundleEnvKeyForHost converts a host into Bundler's per-host credential env var name, +// BundleEnvKeyForHost converts a host into Bundler's per-host credential env var name, // following Bundler's key normalization: uppercase, "." → "__", "-" → "___", and any // remaining non-alphanumeric character → "_", prefixed with "BUNDLE_". // // "mycompany.jfrog.io" → "BUNDLE_MYCOMPANY__JFROG__IO" -func bundleEnvKeyForHost(host string) string { +func BundleEnvKeyForHost(host string) string { key := strings.ToUpper(host) key = strings.ReplaceAll(key, ".", "__") key = strings.ReplaceAll(key, "-", "___") diff --git a/artifactory/commands/ruby/native_ruby_test.go b/artifactory/commands/ruby/native_ruby_test.go index 60f27643..1fa265b1 100644 --- a/artifactory/commands/ruby/native_ruby_test.go +++ b/artifactory/commands/ruby/native_ruby_test.go @@ -21,7 +21,7 @@ func TestBundleEnvKeyForHost(t *testing.T) { {"artifactory", "BUNDLE_ARTIFACTORY"}, } for _, c := range cases { - assert.Equal(t, c.want, bundleEnvKeyForHost(c.host), "host %q", c.host) + assert.Equal(t, c.want, BundleEnvKeyForHost(c.host), "host %q", c.host) } } diff --git a/artifactory/commands/setup/setup.go b/artifactory/commands/setup/setup.go index 71123674..62c254f2 100644 --- a/artifactory/commands/setup/setup.go +++ b/artifactory/commands/setup/setup.go @@ -32,6 +32,7 @@ import ( "github.com/jfrog/jfrog-client-go/utils/errorutils" "github.com/jfrog/jfrog-client-go/utils/log" "golang.org/x/exp/maps" + "gopkg.in/yaml.v3" ) // packageManagerToRepositoryPackageType maps project types to corresponding Artifactory repository package types. @@ -575,10 +576,14 @@ func (sc *SetupCommand) configureUV() error { return nil } +// rubygemsDefaultSource is the default source RubyGems ships with; when present in +// ~/.gemrc's :sources: list, it is always kept first. +const rubygemsDefaultSource = "https://rubygems.org" + // configureRuby configures RubyGems and Bundler to use Artifactory as a gem source. // It performs: -// 1. `bundle config set :` (Bundler per-host credentials) -// 2. Adds the Artifactory source to `~/.gemrc` or prints guidance for Gemfile +// 1. Writes per-host Bundler credentials directly to ~/.bundle/config +// 2. Adds the Artifactory source to ~/.gemrc, or prints guidance for Gemfile // // Both gem and bundle tools will then authenticate to the Artifactory gems repository. func (sc *SetupCommand) configureRuby() error { @@ -598,51 +603,121 @@ func (sc *SetupCommand) configureRuby() error { host += ":" + repoUrl.Port() } - // Configure Bundler: `bundle config set :` - bundleCmd := exec.Command("bundle", "config", "set", host, username+":"+password) - bundleCmd.Stdout = io.Discard - bundleCmd.Stderr = os.Stderr - if bundleErr := bundleCmd.Run(); bundleErr != nil { - log.Warn("Failed to configure Bundler credentials (bundle may not be installed): " + bundleErr.Error()) - } else { - log.Info(fmt.Sprintf("Bundler configured: credentials set for host '%s'", host)) + if bundleErr := writeBundleConfig(host, username, password); bundleErr != nil { + return fmt.Errorf("failed to configure Bundler credentials: %w", bundleErr) } + log.Info(fmt.Sprintf("Bundler configured: credentials set for host '%s'", host)) - // Configure gem: add source to ~/.gemrc if not already present. + // Configure gem: add source to ~/.gemrc. sourceURL := repoUrl.String() - if gemrcErr := rubyAddSourceToGemrc(sourceURL); gemrcErr != nil { - log.Debug("Could not update ~/.gemrc: " + gemrcErr.Error()) + if gemrcErr := addGemrcSource(sourceURL); gemrcErr != nil { + return fmt.Errorf("failed to update ~/.gemrc: %w", gemrcErr) } log.Output(fmt.Sprintf("\nAdd this source to your Gemfile:\n source \"%s\"\n", sourceURL)) return nil } -// rubyAddSourceToGemrc adds the Artifactory gems URL to ~/.gemrc :sources if not present. -func rubyAddSourceToGemrc(sourceURL string) error { +// writeBundleConfig writes per-host Bundler credentials directly to ~/.bundle/config, +// bypassing the `bundle config` CLI entirely (its `set` subcommand doesn't exist on +// Bundler 1.x). The config key is derived via ruby.BundleEnvKeyForHost, the same +// normalization `jf ruby bundle install` uses at runtime, so setup-time and +// runtime-injection key formats can never drift apart. +func writeBundleConfig(host, user, password string) error { + home, err := os.UserHomeDir() + if err != nil { + return err + } + bundleDir := home + "/.bundle" + configPath := bundleDir + "/config" + + existing, readErr := os.ReadFile(configPath) + if readErr != nil && !os.IsNotExist(readErr) { + return readErr + } + + config := map[string]interface{}{} + if len(existing) > 0 { + if unmarshalErr := yaml.Unmarshal(existing, &config); unmarshalErr != nil { + return fmt.Errorf("parse existing %s: %w", configPath, unmarshalErr) + } + } + + config[ruby.BundleEnvKeyForHost(host)] = user + ":" + password + + out, marshalErr := yaml.Marshal(config) + if marshalErr != nil { + return marshalErr + } + if mkdirErr := os.MkdirAll(bundleDir, 0755); mkdirErr != nil { + return mkdirErr + } + return os.WriteFile(configPath, out, 0600) +} + +// addGemrcSource adds sourceURL to ~/.gemrc's :sources: list. An exact-match existing +// entry is moved to the front (behind rubygemsDefaultSource, if present) rather than +// duplicated; a new entry is prepended. Different repos configured across multiple runs +// are meant to coexist here, since gem install natively searches every listed source. +func addGemrcSource(sourceURL string) error { home, err := os.UserHomeDir() if err != nil { return err } gemrcPath := home + "/.gemrc" - // Read existing content. - existing, _ := os.ReadFile(gemrcPath) - content := string(existing) + existing, readErr := os.ReadFile(gemrcPath) + if readErr != nil && !os.IsNotExist(readErr) { + return readErr + } - // If the source is already there, skip. - if strings.Contains(content, sourceURL) { - return nil + config := map[string]interface{}{} + if len(existing) > 0 { + if unmarshalErr := yaml.Unmarshal(existing, &config); unmarshalErr != nil { + return fmt.Errorf("parse existing %s: %w", gemrcPath, unmarshalErr) + } } - // Append a :sources entry. gemrc is YAML-like but simple enough to append. - if !strings.Contains(content, ":sources:") { - content += "\n:sources:\n- https://rubygems.org\n- " + sourceURL + "\n" + var currentSources []string + if raw, ok := config[":sources"]; ok { + if rawList, ok := raw.([]interface{}); ok { + for _, item := range rawList { + if s, ok := item.(string); ok { + currentSources = append(currentSources, s) + } + } + } } else { - content += "- " + sourceURL + "\n" + currentSources = []string{rubygemsDefaultSource} } - return os.WriteFile(gemrcPath, []byte(content), 0644) + config[":sources"] = reorderGemrcSources(currentSources, sourceURL) + + out, marshalErr := yaml.Marshal(config) + if marshalErr != nil { + return marshalErr + } + return os.WriteFile(gemrcPath, out, 0644) +} + +// reorderGemrcSources returns sources with sourceURL moved to the front (deduplicated if +// already present), keeping rubygemsDefaultSource first when it's in the list. +func reorderGemrcSources(sources []string, sourceURL string) []string { + hasDefault := slices.Contains(sources, rubygemsDefaultSource) + + result := make([]string, 0, len(sources)+1) + if hasDefault { + result = append(result, rubygemsDefaultSource) + } + result = append(result, sourceURL) + + for _, s := range sources { + if s == rubygemsDefaultSource || s == sourceURL { + continue + } + result = append(result, s) + } + return result } // configureHelm configures Helm to use Artifactory as an OCI registry. diff --git a/artifactory/commands/setup/setup_test.go b/artifactory/commands/setup/setup_test.go index c02ec8d2..b9d0a2c2 100644 --- a/artifactory/commands/setup/setup_test.go +++ b/artifactory/commands/setup/setup_test.go @@ -23,6 +23,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/exp/slices" + "gopkg.in/yaml.v3" ) const ( @@ -923,3 +924,148 @@ func TestSetupCommand_MavenCorrupted(t *testing.T) { assert.NotContains(t, content, testCredential(), "Old token should be replaced") }) } + +func withTempHome(t *testing.T) string { + origHome := os.Getenv("HOME") + tmpHome := t.TempDir() + os.Setenv("HOME", tmpHome) + t.Cleanup(func() { os.Setenv("HOME", origHome) }) + return tmpHome +} + +func TestWriteBundleConfig_EmptyFile(t *testing.T) { + tmpHome := withTempHome(t) + + require.NoError(t, writeBundleConfig("my.jfrog.io", "admin", "secret")) + + content, err := os.ReadFile(filepath.Join(tmpHome, ".bundle", "config")) + require.NoError(t, err) + assert.Contains(t, string(content), "BUNDLE_MY__JFROG__IO: admin:secret") +} + +func TestWriteBundleConfig_PreservesExistingKeys(t *testing.T) { + tmpHome := withTempHome(t) + + bundleDir := filepath.Join(tmpHome, ".bundle") + require.NoError(t, os.MkdirAll(bundleDir, 0755)) + existing := "BUNDLE_PATH: \"vendor/bundle\"\nBUNDLE_OTHERHOST__COM: other:creds\n" + require.NoError(t, os.WriteFile(filepath.Join(bundleDir, "config"), []byte(existing), 0600)) + + require.NoError(t, writeBundleConfig("my.jfrog.io", "admin", "secret")) + + content, err := os.ReadFile(filepath.Join(bundleDir, "config")) + require.NoError(t, err) + assert.Contains(t, string(content), "BUNDLE_PATH: vendor/bundle") + assert.Contains(t, string(content), "BUNDLE_OTHERHOST__COM: other:creds") + assert.Contains(t, string(content), "BUNDLE_MY__JFROG__IO: admin:secret") +} + +func TestWriteBundleConfig_OverwritesSameHost(t *testing.T) { + tmpHome := withTempHome(t) + + require.NoError(t, writeBundleConfig("my.jfrog.io", "admin", "old-secret")) + require.NoError(t, writeBundleConfig("my.jfrog.io", "admin", "new-secret")) + + configPath := filepath.Join(tmpHome, ".bundle", "config") + content, err := os.ReadFile(configPath) + require.NoError(t, err) + assert.Equal(t, 1, strings.Count(string(content), "BUNDLE_MY__JFROG__IO"), "should have exactly one entry for the host") + assert.Contains(t, string(content), "BUNDLE_MY__JFROG__IO: admin:new-secret") + assert.NotContains(t, string(content), "old-secret") +} + +func TestWriteBundleConfig_MalformedExistingFileErrors(t *testing.T) { + tmpHome := withTempHome(t) + + bundleDir := filepath.Join(tmpHome, ".bundle") + require.NoError(t, os.MkdirAll(bundleDir, 0755)) + malformed := "not: valid: yaml: [unterminated" + configPath := filepath.Join(bundleDir, "config") + require.NoError(t, os.WriteFile(configPath, []byte(malformed), 0600)) + + err := writeBundleConfig("my.jfrog.io", "admin", "secret") + require.Error(t, err) + + // File must not be clobbered. + content, readErr := os.ReadFile(configPath) + require.NoError(t, readErr) + assert.Equal(t, malformed, string(content)) +} + +func TestAddGemrcSource_EmptyFile(t *testing.T) { + tmpHome := withTempHome(t) + + require.NoError(t, addGemrcSource("https://my.jfrog.io/artifactory/api/gems/gems-local")) + + content, err := os.ReadFile(filepath.Join(tmpHome, ".gemrc")) + require.NoError(t, err) + + var parsed map[string]interface{} + require.NoError(t, yaml.Unmarshal(content, &parsed)) + sources, ok := parsed[":sources"].([]interface{}) + require.True(t, ok) + assert.Equal(t, []interface{}{"https://rubygems.org", "https://my.jfrog.io/artifactory/api/gems/gems-local"}, sources) +} + +func TestAddGemrcSource_PreservesUnrelatedKeys(t *testing.T) { + tmpHome := withTempHome(t) + + existing := ":ssl_ca_cert: /etc/ssl/certs/ca.pem\n:sources:\n- https://rubygems.org\n" + require.NoError(t, os.WriteFile(filepath.Join(tmpHome, ".gemrc"), []byte(existing), 0644)) + + require.NoError(t, addGemrcSource("https://my.jfrog.io/artifactory/api/gems/gems-local")) + + content, err := os.ReadFile(filepath.Join(tmpHome, ".gemrc")) + require.NoError(t, err) + assert.Contains(t, string(content), ":ssl_ca_cert: /etc/ssl/certs/ca.pem") +} + +func TestAddGemrcSource_ReAddSameSourceMovesToFrontNoDuplicate(t *testing.T) { + tmpHome := withTempHome(t) + + sourceURL := "https://my.jfrog.io/artifactory/api/gems/gems-local" + require.NoError(t, addGemrcSource(sourceURL)) + require.NoError(t, addGemrcSource(sourceURL)) + + content, err := os.ReadFile(filepath.Join(tmpHome, ".gemrc")) + require.NoError(t, err) + + var parsed map[string]interface{} + require.NoError(t, yaml.Unmarshal(content, &parsed)) + sources, ok := parsed[":sources"].([]interface{}) + require.True(t, ok) + assert.Equal(t, []interface{}{"https://rubygems.org", sourceURL}, sources, "no duplicate entry") +} + +func TestAddGemrcSource_SecondDifferentRepoKeepsBothMostRecentFirst(t *testing.T) { + tmpHome := withTempHome(t) + + firstURL := "https://my.jfrog.io/artifactory/api/gems/gems-local" + secondURL := "https://my.jfrog.io/artifactory/api/gems/gems-local-2" + require.NoError(t, addGemrcSource(firstURL)) + require.NoError(t, addGemrcSource(secondURL)) + + content, err := os.ReadFile(filepath.Join(tmpHome, ".gemrc")) + require.NoError(t, err) + + var parsed map[string]interface{} + require.NoError(t, yaml.Unmarshal(content, &parsed)) + sources, ok := parsed[":sources"].([]interface{}) + require.True(t, ok) + assert.Equal(t, []interface{}{"https://rubygems.org", secondURL, firstURL}, sources, "most recently configured source should be first") +} + +func TestAddGemrcSource_MalformedExistingFileErrors(t *testing.T) { + tmpHome := withTempHome(t) + + malformed := "not: valid: yaml: [unterminated" + gemrcPath := filepath.Join(tmpHome, ".gemrc") + require.NoError(t, os.WriteFile(gemrcPath, []byte(malformed), 0644)) + + err := addGemrcSource("https://my.jfrog.io/artifactory/api/gems/gems-local") + require.Error(t, err) + + content, readErr := os.ReadFile(gemrcPath) + require.NoError(t, readErr) + assert.Equal(t, malformed, string(content)) +} diff --git a/go.mod b/go.mod index 9f407b1e..51a9eacd 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,7 @@ require ( golang.org/x/exp v0.0.0-20260527015227-08cc5374adb3 golang.org/x/mod v0.36.0 gopkg.in/ini.v1 v1.67.1 + gopkg.in/yaml.v3 v3.0.1 helm.sh/helm/v3 v3.19.2 oras.land/oras-go/v2 v2.6.2 ) @@ -192,7 +193,6 @@ require ( google.golang.org/grpc v1.79.3 // indirect google.golang.org/protobuf v1.36.11 // indirect gopkg.in/warnings.v0 v0.1.2 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/client-go v0.34.0 // indirect k8s.io/klog/v2 v2.140.0 // indirect sigs.k8s.io/yaml v1.6.0 // indirect From 9d8fcf679c3a9f52b7b5dc01ec36f40cb9eac746 Mon Sep 17 00:00:00 2001 From: agrasth Date: Fri, 31 Jul 2026 04:18:28 +0530 Subject: [PATCH 13/24] feat: make jf setup ruby fully transparent and version-proof Brings `jf setup ruby` to parity with cargo/nuget/apt, where setup alone is enough for the native tool to resolve and authenticate with no manifest edit. Verified end-to-end against a private Artifactory repo with real Bundler 1.17.2 and RubyGems. Bundler mirror, so no Gemfile edit is needed ~/.bundle/config now gets a `mirror.https://rubygems.org` entry pointing at the Artifactory repository, the Bundler analogue of Cargo's `[source.crates-io] replace-with`. An unmodified `source "https://rubygems.org"` Gemfile resolves through Artifactory, and the lockfile still records rubygems.org, so it stays portable and does not leak the instance URL. Verified present in Bundler 1.17 through 4.0; fallback_timeout defaults to 0, so no TCP probe runs. Fix: bare `gem install` could not authenticate (401) RubyGems has no credential store for installs, so a credential-free source in ~/.gemrc meant every install against a private repo failed with 401. The ~/.gemrc source now embeds credentials, and the file is written 0600. Entries are matched with credentials stripped, so rotating a token replaces the stale entry instead of leaving `gem install` retrying a dead credential. Fix: credential key was wrong for dashed and ported hosts Bundler's key normalization changed between majors: 1.x keeps dashes, 2.x+ turns them into "___". We only ever wrote one spelling, so a dashed host such as my-company.jfrog.io silently failed to authenticate on one of them. BundleCredentialKeys now emits every spelling Bundler may look under, and is shared by setup and the runtime injection in `jf ruby bundle` so the two cannot drift. Setup also keys on the hostname rather than host:port, because Bundler's fallback uses uri.host, which excludes the port; the previous host:port key was never read at all. Bundler parses ~/.bundle/config with its own line-based stub serializer rather than a real YAML parser, so tests now assert round-tripping through a port of that parser instead of merely checking the output is valid YAML. --- artifactory/commands/ruby/native_ruby.go | 54 ++++-- artifactory/commands/ruby/native_ruby_test.go | 22 ++- artifactory/commands/setup/setup.go | 161 +++++++++++++----- artifactory/commands/setup/setup_test.go | 159 ++++++++++++++--- 4 files changed, 313 insertions(+), 83 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index 083e17ca..6baf4276 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -379,7 +379,6 @@ func runRubyBinaryCapture(tool string, args, extraEnv []string) (string, error) return buf.String(), err } - // isRubyHelpRequest reports whether the invocation is purely a help request. func isRubyHelpRequest(subCommand string, args []string) bool { if subCommand == "help" { @@ -439,22 +438,28 @@ func (rc *RubyCommand) injectAuth(serverDetails *coreConfig.ServerDetails, sourc switch rc.nativeTool { case toolBundle: cred := fmt.Sprintf("%s:%s", user, pass) - key := BundleEnvKeyForHost(host) - if os.Getenv(key) != "" { - log.Info(fmt.Sprintf("Ruby auth [bundle]: %s already set — respecting existing credentials", key)) - } else { - extraEnv = append(extraEnv, key+"="+cred) - log.Info(fmt.Sprintf("Ruby auth [bundle]: injecting credentials via %s", key)) - } - // Bundler's credential lookup may strip the port from the key (e.g., for - // localhost:8081 it may check BUNDLE_LOCALHOST rather than BUNDLE_LOCALHOST_8081). - // Inject credentials under the hostname-only key as well to cover all versions. - hostOnly := strings.Split(host, ":")[0] - if hostOnly != host { - keyNoPort := BundleEnvKeyForHost(hostOnly) - if os.Getenv(keyNoPort) == "" { - extraEnv = append(extraEnv, keyNoPort+"="+cred) + // Bundler's credential lookup varies by version, and falls back from the + // host:port form to the bare hostname, so inject every candidate spelling. + candidates := BundleCredentialKeys(host) + if hostOnly := strings.Split(host, ":")[0]; hostOnly != host { + candidates = append(candidates, BundleCredentialKeys(hostOnly)...) + } + seen := map[string]bool{} + var injected []string + for _, key := range candidates { + if seen[key] { + continue + } + seen[key] = true + if os.Getenv(key) != "" { + log.Info(fmt.Sprintf("Ruby auth [bundle]: %s already set — respecting existing credentials", key)) + continue } + extraEnv = append(extraEnv, key+"="+cred) + injected = append(injected, key) + } + if len(injected) > 0 { + log.Info("Ruby auth [bundle]: injecting credentials via " + strings.Join(injected, ", ")) } case toolGem: if os.Getenv("GEM_HOST_API_KEY") != "" { @@ -494,6 +499,23 @@ func rubyHasCredentials(user, pass string) bool { // remaining non-alphanumeric character → "_", prefixed with "BUNDLE_". // // "mycompany.jfrog.io" → "BUNDLE_MYCOMPANY__JFROG__IO" +// +// BundleCredentialKeys returns every key spelling Bundler may look credentials up under +// for host, valid both as a ~/.bundle/config key and as an environment variable name. +// +// Bundler's normalization changed between majors. Bundler 1.x replaces "." with "__" and +// upcases, leaving dashes and colons intact; Bundler 2.x and later additionally replace +// "-" with "___", because environment variable names cannot contain dashes. Emitting +// both spellings authenticates on either, and is why a dashed host or a host carrying a +// port yields two keys rather than one. +func BundleCredentialKeys(host string) []string { + keys := []string{BundleEnvKeyForHost(host)} + if legacy := "BUNDLE_" + strings.ToUpper(strings.ReplaceAll(host, ".", "__")); legacy != keys[0] { + keys = append(keys, legacy) + } + return keys +} + func BundleEnvKeyForHost(host string) string { key := strings.ToUpper(host) key = strings.ReplaceAll(key, ".", "__") diff --git a/artifactory/commands/ruby/native_ruby_test.go b/artifactory/commands/ruby/native_ruby_test.go index 1fa265b1..ab3696d7 100644 --- a/artifactory/commands/ruby/native_ruby_test.go +++ b/artifactory/commands/ruby/native_ruby_test.go @@ -32,7 +32,7 @@ func TestRubyExtractRepoKeyFromURL(t *testing.T) { }{ {"https://my.jfrog.io/artifactory/api/gems/gems-local/", "gems-local"}, {"https://my.jfrog.io/api/gems/gems-remote", "gems-remote"}, - {"gems-local", "gems-local"}, // bare key passthrough + {"gems-local", "gems-local"}, // bare key passthrough {"https://rubygems.org/", ""}, // no /api/gems/ segment {"", ""}, } @@ -327,7 +327,6 @@ func TestExtractGemNamesFromArgs(t *testing.T) { assert.Empty(t, names) } - func TestRubyEmbedCredsInHostArg(t *testing.T) { server := &coreConfig.ServerDetails{ User: "myuser", @@ -613,3 +612,22 @@ func TestRubyEnrichDepsChecksums_NeverTrustsLocalCache(t *testing.T) { assert.Empty(t, deps[0].Sha256, "checksum must not be populated without a verified Artifactory lookup") assert.Empty(t, deps[0].Md5, "checksum must not be populated without a verified Artifactory lookup") } + +// TestBundleCredentialKeys pins the key spellings against what real Bundler computes. +// Verified against Bundler 1.17.2 and 4.0.16: 1.x leaves dashes and colons intact, while +// 2.x+ turns dashes into "___". Both spellings must be emitted or one major silently +// fails to authenticate. +func TestBundleCredentialKeys(t *testing.T) { + // Plain host: a single spelling, identical on every Bundler version. + assert.Equal(t, []string{"BUNDLE_ACME__JFROG__IO"}, BundleCredentialKeys("acme.jfrog.io")) + + // Dashed host: Bundler 2.x+ wants "___", Bundler 1.x wants the dash preserved. + assert.Equal(t, + []string{"BUNDLE_MY___COMPANY__JFROG__IO", "BUNDLE_MY-COMPANY__JFROG__IO"}, + BundleCredentialKeys("my-company.jfrog.io")) + + // Ported host: Bundler preserves the colon, which the env-var spelling cannot. + assert.Equal(t, + []string{"BUNDLE_LOCALHOST_8081", "BUNDLE_LOCALHOST:8081"}, + BundleCredentialKeys("localhost:8081")) +} diff --git a/artifactory/commands/setup/setup.go b/artifactory/commands/setup/setup.go index 62c254f2..d05e543d 100644 --- a/artifactory/commands/setup/setup.go +++ b/artifactory/commands/setup/setup.go @@ -1,12 +1,14 @@ package setup import ( + "bytes" _ "embed" "fmt" "io" "net/url" "os" "os/exec" + "path/filepath" "slices" "strings" @@ -576,60 +578,90 @@ func (sc *SetupCommand) configureUV() error { return nil } -// rubygemsDefaultSource is the default source RubyGems ships with; when present in -// ~/.gemrc's :sources: list, it is always kept first. +// rubygemsDefaultSource is the public source that RubyGems and Bundler use by default. +// It stays first in ~/.gemrc's :sources: list, and is the source mirrored to Artifactory +// so that unmodified Gemfiles resolve through Artifactory. const rubygemsDefaultSource = "https://rubygems.org" -// configureRuby configures RubyGems and Bundler to use Artifactory as a gem source. -// It performs: -// 1. Writes per-host Bundler credentials directly to ~/.bundle/config -// 2. Adds the Artifactory source to ~/.gemrc, or prints guidance for Gemfile +// configureRuby points RubyGems and Bundler at Artifactory, so that plain `gem` and +// `bundle` commands resolve and authenticate through it with no edit to the Gemfile. // -// Both gem and bundle tools will then authenticate to the Artifactory gems repository. +// Everything is written by editing the config files directly, never by shelling out to +// `gem`/`bundle`, because their CLI syntax differs across versions (notably +// `bundle config set`, which does not exist before Bundler 2.0): +// +// 1. ~/.bundle/config — a mirror redirecting https://rubygems.org to the Artifactory +// repository, plus per-host credentials. +// 2. ~/.gemrc — the Artifactory repository added to :sources:, for bare `gem install`. func (sc *SetupCommand) configureRuby() error { repoUrl, username, password, err := ruby.GetRubyGemsRepoUrlWithCredentials(sc.serverDetails, sc.repoName) if err != nil { return fmt.Errorf("failed to get RubyGems repository URL with credentials: %w", err) } - // If no credentials are provided, just print guidance. - if username == "" && password == "" { - log.Output(fmt.Sprintf("Add this source to your Gemfile:\n source \"%s\"\n", repoUrl.String())) - return nil - } - - host := repoUrl.Hostname() - if repoUrl.Port() != "" { - host += ":" + repoUrl.Port() + // sourceURL stays credential-free: it is what gets printed for the user to paste into + // a shared Gemfile. authenticatedURL is the same repository with credentials embedded, + // which is what the local config files need. + sourceURL := repoUrl.String() + authenticatedURL := sourceURL + if password != "" { + withCredentials := *repoUrl + withCredentials.User = url.UserPassword(username, password) + authenticatedURL = withCredentials.String() + } + settings := map[string]string{} + + // Mirror the public RubyGems source to Artifactory, so a Gemfile that says + // `source "https://rubygems.org"` resolves through Artifactory unchanged. Credentials + // are embedded in the mirror value: Bundler keeps a mirror URI's own userinfo instead + // of looking credentials up separately, which behaves identically on every version. + settings[bundleMirrorKey(rubygemsDefaultSource)] = authenticatedURL + + // Per-host credentials, for Gemfiles that name the Artifactory source explicitly. + if password != "" { + credential := username + ":" + password + for _, key := range ruby.BundleCredentialKeys(repoUrl.Hostname()) { + settings[key] = credential + } } - if bundleErr := writeBundleConfig(host, username, password); bundleErr != nil { - return fmt.Errorf("failed to configure Bundler credentials: %w", bundleErr) + if bundleErr := writeBundleSettings(settings); bundleErr != nil { + return fmt.Errorf("failed to configure Bundler: %w", bundleErr) } - log.Info(fmt.Sprintf("Bundler configured: credentials set for host '%s'", host)) + log.Info(fmt.Sprintf("Bundler configured: %s is mirrored to %s", rubygemsDefaultSource, sourceURL)) - // Configure gem: add source to ~/.gemrc. - sourceURL := repoUrl.String() - if gemrcErr := addGemrcSource(sourceURL); gemrcErr != nil { + if gemrcErr := addGemrcSource(authenticatedURL); gemrcErr != nil { return fmt.Errorf("failed to update ~/.gemrc: %w", gemrcErr) } + log.Info("RubyGems configured: source added to ~/.gemrc") - log.Output(fmt.Sprintf("\nAdd this source to your Gemfile:\n source \"%s\"\n", sourceURL)) + log.Output(fmt.Sprintf( + "\nBundler and RubyGems now resolve through Artifactory.\n"+ + " A Gemfile using `source \"%s\"` needs no change.\n"+ + " To depend on this repository explicitly, use:\n source \"%s\"\n", + rubygemsDefaultSource, sourceURL)) return nil } -// writeBundleConfig writes per-host Bundler credentials directly to ~/.bundle/config, -// bypassing the `bundle config` CLI entirely (its `set` subcommand doesn't exist on -// Bundler 1.x). The config key is derived via ruby.BundleEnvKeyForHost, the same -// normalization `jf ruby bundle install` uses at runtime, so setup-time and -// runtime-injection key formats can never drift apart. -func writeBundleConfig(host, user, password string) error { +// bundleMirrorKey returns the ~/.bundle/config key Bundler reads a mirror from for the +// given upstream source. Bundler builds it from "mirror." by normalizing the URI to +// a trailing slash, replacing "." with "__", and upcasing: +// +// https://rubygems.org → BUNDLE_MIRROR__HTTPS://RUBYGEMS__ORG/ +func bundleMirrorKey(sourceURL string) string { + normalized := strings.TrimSuffix(sourceURL, "/") + "/" + return "BUNDLE_" + strings.ToUpper(strings.ReplaceAll("mirror."+normalized, ".", "__")) +} + +// writeBundleSettings merges entries into ~/.bundle/config, preserving every setting +// already present. The file holds credentials, so it is written 0600. +func writeBundleSettings(entries map[string]string) error { home, err := os.UserHomeDir() if err != nil { return err } - bundleDir := home + "/.bundle" - configPath := bundleDir + "/config" + bundleDir := filepath.Join(home, ".bundle") + configPath := filepath.Join(bundleDir, "config") existing, readErr := os.ReadFile(configPath) if readErr != nil && !os.IsNotExist(readErr) { @@ -642,10 +674,11 @@ func writeBundleConfig(host, user, password string) error { return fmt.Errorf("parse existing %s: %w", configPath, unmarshalErr) } } + for key, value := range entries { + config[key] = value + } - config[ruby.BundleEnvKeyForHost(host)] = user + ":" + password - - out, marshalErr := yaml.Marshal(config) + out, marshalErr := marshalBundleConfig(config) if marshalErr != nil { return marshalErr } @@ -655,16 +688,37 @@ func writeBundleConfig(host, user, password string) error { return os.WriteFile(configPath, out, 0600) } -// addGemrcSource adds sourceURL to ~/.gemrc's :sources: list. An exact-match existing -// entry is moved to the front (behind rubygemsDefaultSource, if present) rather than -// duplicated; a new entry is prepended. Different repos configured across multiple runs -// are meant to coexist here, since gem install natively searches every listed source. +// marshalBundleConfig renders Bundler's config as YAML that Bundler's own parser accepts. +// Bundler reads this file with a line-based stub serializer rather than a real YAML +// parser: it needs each setting on a single line, and it measures nesting depth in +// two-space units. +func marshalBundleConfig(config map[string]interface{}) ([]byte, error) { + var buf bytes.Buffer + encoder := yaml.NewEncoder(&buf) + encoder.SetIndent(2) + if err := encoder.Encode(config); err != nil { + return nil, err + } + if err := encoder.Close(); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +// addGemrcSource adds sourceURL to ~/.gemrc's :sources: list, moving it to the front +// (behind rubygemsDefaultSource, if present) so `gem install` tries it first. Different +// repositories configured across separate runs are meant to coexist here, because +// `gem install` natively searches every listed source. +// +// sourceURL embeds credentials when the server has them: unlike Bundler, RubyGems has no +// separate credential store for installing, so the source URL is the only way a plain +// `gem install` can authenticate. That is why the file is written 0600. func addGemrcSource(sourceURL string) error { home, err := os.UserHomeDir() if err != nil { return err } - gemrcPath := home + "/.gemrc" + gemrcPath := filepath.Join(home, ".gemrc") existing, readErr := os.ReadFile(gemrcPath) if readErr != nil && !os.IsNotExist(readErr) { @@ -697,13 +751,31 @@ func addGemrcSource(sourceURL string) error { if marshalErr != nil { return marshalErr } - return os.WriteFile(gemrcPath, out, 0644) + // The source URL may embed credentials, so this file must not be world-readable. + return os.WriteFile(gemrcPath, out, 0600) } -// reorderGemrcSources returns sources with sourceURL moved to the front (deduplicated if -// already present), keeping rubygemsDefaultSource first when it's in the list. +// gemSourceIdentity strips embedded credentials and any trailing slash from a gem source +// URL, so that two entries pointing at the same repository compare equal even when their +// credentials differ. Without this, re-running setup after a token rotation would leave +// the stale entry behind and `gem install` would keep trying the old credentials. +func gemSourceIdentity(rawURL string) string { + parsed, err := url.Parse(rawURL) + if err != nil { + return strings.TrimSuffix(rawURL, "/") + } + parsed.User = nil + return strings.TrimSuffix(parsed.String(), "/") +} + +// reorderGemrcSources returns sources with sourceURL moved to the front, replacing any +// existing entry for the same repository, and keeping rubygemsDefaultSource first when +// it is in the list. func reorderGemrcSources(sources []string, sourceURL string) []string { - hasDefault := slices.Contains(sources, rubygemsDefaultSource) + target := gemSourceIdentity(sourceURL) + hasDefault := slices.ContainsFunc(sources, func(s string) bool { + return gemSourceIdentity(s) == rubygemsDefaultSource + }) result := make([]string, 0, len(sources)+1) if hasDefault { @@ -712,7 +784,8 @@ func reorderGemrcSources(sources []string, sourceURL string) []string { result = append(result, sourceURL) for _, s := range sources { - if s == rubygemsDefaultSource || s == sourceURL { + identity := gemSourceIdentity(s) + if identity == rubygemsDefaultSource || identity == target { continue } result = append(result, s) diff --git a/artifactory/commands/setup/setup_test.go b/artifactory/commands/setup/setup_test.go index b9d0a2c2..a78bfd4f 100644 --- a/artifactory/commands/setup/setup_test.go +++ b/artifactory/commands/setup/setup_test.go @@ -933,48 +933,104 @@ func withTempHome(t *testing.T) string { return tmpHome } -func TestWriteBundleConfig_EmptyFile(t *testing.T) { +// bundlerParseConfig mirrors how Bundler itself reads ~/.bundle/config. Bundler uses a +// line-based stub serializer rather than a YAML parser: its HASH_REGEX takes the key up +// to the last colon followed by whitespace or end-of-line, then strips one optional pair +// of surrounding quotes from the value. Porting that here lets these tests assert that +// what we write is what Bundler actually reads back, rather than merely that it is valid +// YAML. +func bundlerParseConfig(content string) map[string]string { + parsed := map[string]string{} + for _, line := range strings.Split(content, "\n") { + separator := -1 + for i := 0; i < len(line); i++ { + if line[i] != ':' { + continue + } + if i+1 == len(line) || line[i+1] == ' ' || line[i+1] == '\t' { + separator = i + } + } + if separator < 0 { + continue + } + key := strings.TrimLeft(line[:separator], " ") + value := strings.TrimPrefix(line[separator+1:], " ") + if len(value) >= 2 && (value[0] == '"' || value[0] == '\'') && value[len(value)-1] == value[0] { + value = value[1 : len(value)-1] + } + if key == "" || value == "" { + continue + } + parsed[key] = value + } + return parsed +} + +func readBundleConfig(t *testing.T, home string) map[string]string { + t.Helper() + content, err := os.ReadFile(filepath.Join(home, ".bundle", "config")) + require.NoError(t, err) + return bundlerParseConfig(string(content)) +} + +// TestBundleMirrorKey pins the mirror key against the value real Bundler computes for +// "mirror.https://rubygems.org" (verified against Bundler 1.17 and 4.0). If this drifts, +// Bundler silently stops redirecting to Artifactory. +func TestBundleMirrorKey(t *testing.T) { + assert.Equal(t, "BUNDLE_MIRROR__HTTPS://RUBYGEMS__ORG/", bundleMirrorKey("https://rubygems.org")) + assert.Equal(t, "BUNDLE_MIRROR__HTTPS://RUBYGEMS__ORG/", bundleMirrorKey("https://rubygems.org/")) +} + +func TestWriteBundleSettings_EmptyFile(t *testing.T) { tmpHome := withTempHome(t) - require.NoError(t, writeBundleConfig("my.jfrog.io", "admin", "secret")) + require.NoError(t, writeBundleSettings(map[string]string{"BUNDLE_MY__JFROG__IO": "admin:secret"})) - content, err := os.ReadFile(filepath.Join(tmpHome, ".bundle", "config")) - require.NoError(t, err) - assert.Contains(t, string(content), "BUNDLE_MY__JFROG__IO: admin:secret") + assert.Equal(t, "admin:secret", readBundleConfig(t, tmpHome)["BUNDLE_MY__JFROG__IO"]) } -func TestWriteBundleConfig_PreservesExistingKeys(t *testing.T) { +func TestWriteBundleSettings_PreservesExistingKeys(t *testing.T) { tmpHome := withTempHome(t) bundleDir := filepath.Join(tmpHome, ".bundle") require.NoError(t, os.MkdirAll(bundleDir, 0755)) - existing := "BUNDLE_PATH: \"vendor/bundle\"\nBUNDLE_OTHERHOST__COM: other:creds\n" + // Written the way Bundler itself writes it, with quoted values. + existing := "---\nBUNDLE_PATH: \"vendor/bundle\"\nBUNDLE_OTHERHOST__COM: \"other:creds\"\n" require.NoError(t, os.WriteFile(filepath.Join(bundleDir, "config"), []byte(existing), 0600)) - require.NoError(t, writeBundleConfig("my.jfrog.io", "admin", "secret")) + require.NoError(t, writeBundleSettings(map[string]string{"BUNDLE_MY__JFROG__IO": "admin:secret"})) - content, err := os.ReadFile(filepath.Join(bundleDir, "config")) - require.NoError(t, err) - assert.Contains(t, string(content), "BUNDLE_PATH: vendor/bundle") - assert.Contains(t, string(content), "BUNDLE_OTHERHOST__COM: other:creds") - assert.Contains(t, string(content), "BUNDLE_MY__JFROG__IO: admin:secret") + parsed := readBundleConfig(t, tmpHome) + assert.Equal(t, "vendor/bundle", parsed["BUNDLE_PATH"]) + assert.Equal(t, "other:creds", parsed["BUNDLE_OTHERHOST__COM"]) + assert.Equal(t, "admin:secret", parsed["BUNDLE_MY__JFROG__IO"]) } -func TestWriteBundleConfig_OverwritesSameHost(t *testing.T) { +func TestWriteBundleSettings_OverwritesSameKey(t *testing.T) { tmpHome := withTempHome(t) - require.NoError(t, writeBundleConfig("my.jfrog.io", "admin", "old-secret")) - require.NoError(t, writeBundleConfig("my.jfrog.io", "admin", "new-secret")) + require.NoError(t, writeBundleSettings(map[string]string{"BUNDLE_MY__JFROG__IO": "admin:old-secret"})) + require.NoError(t, writeBundleSettings(map[string]string{"BUNDLE_MY__JFROG__IO": "admin:new-secret"})) - configPath := filepath.Join(tmpHome, ".bundle", "config") - content, err := os.ReadFile(configPath) + content, err := os.ReadFile(filepath.Join(tmpHome, ".bundle", "config")) require.NoError(t, err) assert.Equal(t, 1, strings.Count(string(content), "BUNDLE_MY__JFROG__IO"), "should have exactly one entry for the host") - assert.Contains(t, string(content), "BUNDLE_MY__JFROG__IO: admin:new-secret") assert.NotContains(t, string(content), "old-secret") + assert.Equal(t, "admin:new-secret", bundlerParseConfig(string(content))["BUNDLE_MY__JFROG__IO"]) +} + +func TestWriteBundleSettings_FileIsPrivate(t *testing.T) { + tmpHome := withTempHome(t) + + require.NoError(t, writeBundleSettings(map[string]string{"BUNDLE_MY__JFROG__IO": "admin:secret"})) + + info, err := os.Stat(filepath.Join(tmpHome, ".bundle", "config")) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0600), info.Mode().Perm(), "config holds credentials and must not be world-readable") } -func TestWriteBundleConfig_MalformedExistingFileErrors(t *testing.T) { +func TestWriteBundleSettings_MalformedExistingFileErrors(t *testing.T) { tmpHome := withTempHome(t) bundleDir := filepath.Join(tmpHome, ".bundle") @@ -983,7 +1039,7 @@ func TestWriteBundleConfig_MalformedExistingFileErrors(t *testing.T) { configPath := filepath.Join(bundleDir, "config") require.NoError(t, os.WriteFile(configPath, []byte(malformed), 0600)) - err := writeBundleConfig("my.jfrog.io", "admin", "secret") + err := writeBundleSettings(map[string]string{"BUNDLE_MY__JFROG__IO": "admin:secret"}) require.Error(t, err) // File must not be clobbered. @@ -992,6 +1048,30 @@ func TestWriteBundleConfig_MalformedExistingFileErrors(t *testing.T) { assert.Equal(t, malformed, string(content)) } +// TestWriteBundleSettings_BundlerReadsMirrorAndCredentials is the end-to-end guarantee: +// the mirror key contains "://" and a trailing slash, and the mirror value contains +// embedded credentials with their own colons. Asserting through Bundler's own parsing +// rules proves none of that confuses the key/value split. +func TestWriteBundleSettings_BundlerReadsMirrorAndCredentials(t *testing.T) { + tmpHome := withTempHome(t) + + mirrorKey := bundleMirrorKey(rubygemsDefaultSource) + mirrorValue := "https://admin:p%40ss%3Aword@acme.jfrog.io/artifactory/api/gems/gems-remote" + require.NoError(t, writeBundleSettings(map[string]string{ + mirrorKey: mirrorValue, + "BUNDLE_ACME__JFROG__IO": "admin:p%40ss%3Aword", + "BUNDLE_MY-CO__JFROG__IO": "admin:secret", + "BUNDLE_MY___CO__JFROG__IO": "admin:secret", + })) + + parsed := readBundleConfig(t, tmpHome) + assert.Equal(t, mirrorValue, parsed[mirrorKey], "Bundler must read the full mirror URL, credentials included") + assert.Equal(t, "admin:p%40ss%3Aword", parsed["BUNDLE_ACME__JFROG__IO"]) + // Both dash spellings must survive, so Bundler 1.x and 2.x+ each find their own. + assert.Equal(t, "admin:secret", parsed["BUNDLE_MY-CO__JFROG__IO"]) + assert.Equal(t, "admin:secret", parsed["BUNDLE_MY___CO__JFROG__IO"]) +} + func TestAddGemrcSource_EmptyFile(t *testing.T) { tmpHome := withTempHome(t) @@ -1069,3 +1149,40 @@ func TestAddGemrcSource_MalformedExistingFileErrors(t *testing.T) { require.NoError(t, readErr) assert.Equal(t, malformed, string(content)) } + +// TestAddGemrcSource_CredentialRotationReplacesEntry guards the case that would otherwise +// leave `gem install` retrying a dead credential: re-running setup after a token rotation +// must replace the existing entry for that repository, not accumulate a stale one. +func TestAddGemrcSource_CredentialRotationReplacesEntry(t *testing.T) { + tmpHome := withTempHome(t) + + base := "https://acme.jfrog.io/artifactory/api/gems/gems-virtual" + require.NoError(t, addGemrcSource("https://admin:old-token@acme.jfrog.io/artifactory/api/gems/gems-virtual")) + require.NoError(t, addGemrcSource("https://admin:new-token@acme.jfrog.io/artifactory/api/gems/gems-virtual")) + + content, err := os.ReadFile(filepath.Join(tmpHome, ".gemrc")) + require.NoError(t, err) + var parsed map[string]interface{} + require.NoError(t, yaml.Unmarshal(content, &parsed)) + sources, ok := parsed[":sources"].([]interface{}) + require.True(t, ok) + + assert.Equal(t, []interface{}{ + "https://rubygems.org", + "https://admin:new-token@acme.jfrog.io/artifactory/api/gems/gems-virtual", + }, sources, "the rotated credential must replace the old entry for the same repository") + assert.NotContains(t, string(content), "old-token") + assert.Equal(t, base, gemSourceIdentity("https://admin:new-token@acme.jfrog.io/artifactory/api/gems/gems-virtual/")) +} + +// TestAddGemrcSource_FileIsPrivate: the source URL embeds credentials, because RubyGems +// has no other way to authenticate an install, so the file must not be world-readable. +func TestAddGemrcSource_FileIsPrivate(t *testing.T) { + tmpHome := withTempHome(t) + + require.NoError(t, addGemrcSource("https://admin:tok@acme.jfrog.io/artifactory/api/gems/gems-virtual")) + + info, err := os.Stat(filepath.Join(tmpHome, ".gemrc")) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0600), info.Mode().Perm()) +} From 7566504acf9733da25d06e4d164d4a4ac9cb4b4b Mon Sep 17 00:00:00 2001 From: agrasth Date: Fri, 31 Jul 2026 04:33:37 +0530 Subject: [PATCH 14/24] fix: stop leaking Artifactory credentials to unrelated gem hosts The host-match guard that protects against sending Artifactory credentials to a third-party registry lived inside injectAuth and only short-circuited the environment variables. Run() then went on to embed credentials in argv and write ~/.gem/credentials regardless, so the guard was bypassed on exactly the two paths that persist a credential. jf ruby gem install foo --source https://gems.thirdparty.com/api/gems/x/ with no --server-id logged "skipping credential injection" and then passed --source https://user:@gems.thirdparty.com/... to the child process. The gem push path did the same via ~/.gem/credentials. Authorization now lives in authorizedForSource and gates every credential path in Run(), with injectAuth keeping its own check as defence in depth. It is evaluated against the host the native tool will really contact, so `--repo --host ` can no longer authorize against one host and hand the credential to another. Also fixes `gem push` targeting the wrong host entirely: --host was only injected when --repo was passed, so a push whose source came from the Gemfile or ~/.gemrc fell back to RubyGems' default host and sent both the gem and the Artifactory credential to rubygems.org. Two build-info correctness fixes found in the same audit: - AQL enrichment read the checksum from the first matching result but the repository path from the last, so a gem with a platform-specific sibling (nokogiri-1.16.0.gem alongside nokogiri-1.16.0-arm64-darwin.gem) published a checksum for a file the recorded path did not point at. Both now come from one result, and an exact -.gem wins over a platform variant. - `gem install -v "~> 13.0"` recorded the requirement as the version, producing a dependency ID of "rake:~> 13.0" that matches no artifact. Requirements are now rejected so the installed version is queried instead. --- artifactory/commands/ruby/native_ruby.go | 140 +++++++++++++----- artifactory/commands/ruby/native_ruby_test.go | 14 ++ 2 files changed, 118 insertions(+), 36 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index 6baf4276..75f67c48 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -71,18 +71,45 @@ func (rc *RubyCommand) Run() error { // Discover the Artifactory gem source the project points at, then inject auth. sourceURL, repoKey := rc.resolveRepo(workingDir, serverDetails) - // When --repo constructed the URL and no --source/--host was provided in args, - // inject the source/host arg into the native command so the tool knows where to point. - if rc.repository != "" && sourceURL != "" && rubySourceFromArgs(rc.args) == "" { + // Point the native command at the discovered Artifactory source when the user did not + // name one explicitly. For `gem push` this is a correctness requirement rather than a + // convenience: with no --host, RubyGems falls back to its default host + // (https://rubygems.org), so both the gem and the credential would go there instead. + if sourceURL != "" && rubySourceFromArgs(rc.args) == "" && + (rc.repository != "" || (rc.nativeTool == toolGem && subCommand == "push")) { rc.args = rubyInjectSourceArg(rc.nativeTool, subCommand, rc.args, sourceURL) } + // authTarget is the URL credentials would actually reach. For `gem push` an explicit + // --host in the args beats the discovered source, so authorization and the credential + // key must both follow the host the native tool will really contact — otherwise + // `--repo artifactory --host third-party` would authorize against one host and send + // the credential to another. + authTarget := sourceURL + if rc.nativeTool == toolGem && subCommand == "push" { + if explicitHost := rubySourceFromArgs(rc.args); explicitHost != "" { + authTarget = explicitHost + } + } + var extraEnv []string var credCleanup func() + switch { // gem build is a pure local operation — skip auth injection entirely. - if rc.nativeTool == toolGem && subCommand == "build" { + case rc.nativeTool == toolGem && subCommand == "build": log.Debug("Ruby auth: skipping credential injection for gem build (local-only operation)") - } else if serverDetails != nil && sourceURL != "" { + case serverDetails == nil || sourceURL == "": + log.Debug("Ruby auth: no Artifactory gem source discovered in args/Gemfile/gem-sources — skipping credential injection") + // Every credential path below is gated on this one check, not just the environment + // variables: embedding a credential in argv or writing it to ~/.gem/credentials leaks + // it just as effectively, so an unrelated registry must never reach any of them. + case !rc.authorizedForSource(serverDetails, authTarget): + log.Warn(fmt.Sprintf( + "Ruby auth: target host (%s) differs from jf server config host (%s) — "+ + "skipping credential injection. Use --server-id to authenticate explicitly, "+ + "or configure credentials with `bundle config` / ~/.gem/credentials.", + rubyHostOf(authTarget), rubyHostOf(serverDetails.ArtifactoryUrl))) + default: extraEnv = rc.injectAuth(serverDetails, sourceURL) if rc.nativeTool == toolGem { switch subCommand { @@ -98,7 +125,7 @@ func (rc *RubyCommand) Run() error { // Write temporary ~/.gem/credentials for the target host. // CRITICAL: the credentials key MUST exactly match the --host value that // gets passed to the native command (no trailing slash). - pushHost := strings.TrimRight(sourceURL, "/") + pushHost := strings.TrimRight(authTarget, "/") cleanup, credErr := rubyWriteTempGemCredentials(pushHost, serverDetails) if credErr != nil { log.Warn("Ruby auth [gem push]: failed to write temporary credentials: " + credErr.Error()) @@ -108,8 +135,6 @@ func (rc *RubyCommand) Run() error { } } } - } else if serverDetails != nil && sourceURL == "" { - log.Debug("Ruby auth: no Artifactory gem source discovered in args/Gemfile/gem-sources — skipping credential injection") } defer func() { if credCleanup != nil { @@ -403,6 +428,18 @@ func rubyResolveServerDetails(serverID string) (*coreConfig.ServerDetails, error // ── Authentication ─────────────────────────────────────────────────────────── +// authorizedForSource reports whether Artifactory credentials may be sent to targetURL. +// +// Without an explicit --server-id, credentials only ever go to the host the jf server +// config points at, so that a Gemfile, a --source, or a --host naming an unrelated +// registry can never receive them. Passing --server-id is the explicit opt-in. +func (rc *RubyCommand) authorizedForSource(serverDetails *coreConfig.ServerDetails, targetURL string) bool { + if rc.serverID != "" || targetURL == "" { + return true + } + return rubyHostMatchesServer(targetURL, serverDetails.ArtifactoryUrl) +} + // injectAuth returns the additional environment variables required to authenticate // the native tool against Artifactory. It is non-destructive: a credential is only // injected when the user has not already configured one natively (env var, embedded @@ -423,14 +460,10 @@ func (rc *RubyCommand) injectAuth(serverDetails *coreConfig.ServerDetails, sourc if host == "" { host = rubyHostOf(serverDetails.ArtifactoryUrl) } - // Without --server-id, only inject when the source host matches the jf server - // host to avoid leaking credentials to an unrelated registry. - if rc.serverID == "" && sourceURL != "" && !rubyHostMatchesServer(sourceURL, serverDetails.ArtifactoryUrl) { - log.Warn(fmt.Sprintf( - "Ruby auth: gem source host (%s) differs from jf server config host (%s) — "+ - "skipping credential injection. Use --server-id to authenticate explicitly, "+ - "or configure credentials with `bundle config set` / ~/.gem/credentials.", - host, rubyHostOf(serverDetails.ArtifactoryUrl))) + // Defence in depth: Run() already gates every credential path on this same check, but + // injectAuth must never hand out credentials for an unrelated host on its own either. + if !rc.authorizedForSource(serverDetails, sourceURL) { + log.Debug(fmt.Sprintf("Ruby auth: refusing to inject credentials for unrelated host %s", host)) return nil } @@ -950,17 +983,30 @@ func extractVersionFromArgs(args []string) string { for i, a := range args { switch { case (a == "-v" || a == "--version") && i+1 < len(args): - return strings.TrimSpace(args[i+1]) + return exactGemVersion(args[i+1]) case strings.HasPrefix(a, "--version="): - return strings.TrimSpace(strings.TrimPrefix(a, "--version=")) + return exactGemVersion(strings.TrimPrefix(a, "--version=")) case strings.HasPrefix(a, "-v") && len(a) > 2 && a[2] != '-': // -v1.0.0 form (unusual but valid) - return strings.TrimSpace(a[2:]) + return exactGemVersion(a[2:]) } } return "" } +// exactGemVersion returns value only when it is a concrete version. RubyGems equally +// accepts a requirement here ("~> 13.0", ">= 1.2"), which cannot stand in for a version: +// it would produce a build-info dependency ID such as "rake:~> 13.0" that matches no +// artifact in Artifactory. Returning empty instead makes the caller fall back to querying +// the version actually installed. +func exactGemVersion(value string) string { + value = strings.TrimSpace(value) + if value == "" || strings.ContainsAny(value, "~><=,*| ") { + return "" + } + return value +} + // queryInstalledGemVersion queries the installed version of a gem via `gem list --exact `. // Used as a fallback when stdout parsing doesn't yield results. // Returns the latest installed version or empty string if not found. @@ -1375,29 +1421,51 @@ func rubyEnrichDepsViaAQL(deps []buildinfo.Dependency, entries []rubyDepEntry, r return } + // Resolve one AQL result per dependency, then take the checksums and the path from that + // single result. The name pattern is deliberately loose enough to match a + // platform-specific build ("nokogiri-1.16.0-arm64-darwin.gem") as well as the plain + // gem, so a dependency can match several results — the exact "-.gem" + // always wins. Reading the checksum from one result and the path from another would + // publish build-info claiming a checksum for a file it does not point at. enriched := 0 - for _, r := range aqlResult.Results { - if r.ActualSha1 == "" { + for _, e := range entries { + if deps[e.idx].Sha1 != "" { continue } - for _, e := range entries { - if r.Name == e.prefix+".gem" || strings.HasPrefix(r.Name, e.prefix+"-") { - if deps[e.idx].Sha1 == "" { - deps[e.idx].Sha1 = r.ActualSha1 - deps[e.idx].Md5 = r.ActualMd5 - if r.Sha256 != "" && deps[e.idx].Sha256 == "" { - deps[e.idx].Sha256 = r.Sha256 - } - } - if r.Path != "" && r.Path != "." { - deps[e.idx].Repository = searchRepo + "/" + r.Path + "/" + r.Name - } else { - deps[e.idx].Repository = searchRepo + "/" + r.Name - } - enriched++ + var match *struct { + Name string `json:"name"` + Path string `json:"path"` + ActualSha1 string `json:"actual_sha1"` + ActualMd5 string `json:"actual_md5"` + Sha256 string `json:"sha256"` + } + for i := range aqlResult.Results { + candidate := &aqlResult.Results[i] + if candidate.ActualSha1 == "" { + continue + } + if candidate.Name == e.prefix+".gem" { + match = candidate break } + if match == nil && strings.HasPrefix(candidate.Name, e.prefix+"-") { + match = candidate + } + } + if match == nil { + continue + } + deps[e.idx].Sha1 = match.ActualSha1 + deps[e.idx].Md5 = match.ActualMd5 + if match.Sha256 != "" { + deps[e.idx].Sha256 = match.Sha256 + } + if match.Path != "" && match.Path != "." { + deps[e.idx].Repository = searchRepo + "/" + match.Path + "/" + match.Name + } else { + deps[e.idx].Repository = searchRepo + "/" + match.Name } + enriched++ } if enriched > 0 { diff --git a/artifactory/commands/ruby/native_ruby_test.go b/artifactory/commands/ruby/native_ruby_test.go index ab3696d7..5e90449b 100644 --- a/artifactory/commands/ruby/native_ruby_test.go +++ b/artifactory/commands/ruby/native_ruby_test.go @@ -631,3 +631,17 @@ func TestBundleCredentialKeys(t *testing.T) { []string{"BUNDLE_LOCALHOST_8081", "BUNDLE_LOCALHOST:8081"}, BundleCredentialKeys("localhost:8081")) } + +// TestExtractVersionFromArgs_RejectsRequirements guards against recording a version +// requirement as if it were a version, which produced dependency IDs like "rake:~> 13.0" +// that match no artifact in Artifactory. +func TestExtractVersionFromArgs_RejectsRequirements(t *testing.T) { + for _, requirement := range []string{"~> 13.0", ">= 1.2", "< 2", "= 1.0", ">1.0", "1.0, 2.0", "13.*"} { + assert.Empty(t, extractVersionFromArgs([]string{"install", "rake", "-v", requirement}), + "requirement %q must not be treated as a concrete version", requirement) + } + // Concrete versions still resolve, including prereleases. + assert.Equal(t, "13.0.6", extractVersionFromArgs([]string{"install", "rake", "-v", "13.0.6"})) + assert.Equal(t, "7.1.0.beta1", extractVersionFromArgs([]string{"install", "rails", "--version=7.1.0.beta1"})) + assert.Equal(t, "1.0.0", extractVersionFromArgs([]string{"install", "gem", "-v1.0.0"})) +} From 16b4fc3425f0a11fdb3ad3274c7bb572f30e6b39 Mon Sep 17 00:00:00 2001 From: agrasth Date: Fri, 31 Jul 2026 10:37:10 +0530 Subject: [PATCH 15/24] build: require the pinned build-info-go instead of replacing it The build-info-go version the RubyGems code needs was pinned with a replace directive, while the require line still named an older commit. Go ignores replace directives from dependencies, so a consumer resolved the stale require and failed to compile the ruby package with undefined flexpack.GemConfig and buildinfo.Gem. Promoting the pin to the require line makes it propagate through normal minimal version selection, so jfrog-cli no longer needs its own matching replace. The resolved module is byte-identical to what the replace already produced, so this does not change the build of this module. --- go.mod | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 51a9eacd..cdca6f4a 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/forPelevin/gomoji v1.4.1 github.com/google/go-containerregistry v0.21.3 github.com/jedib0t/go-pretty/v6 v6.7.10 - github.com/jfrog/build-info-go v1.13.1-0.20260610071651-260ad6720e0d + github.com/jfrog/build-info-go v1.13.1-0.20260715194847-6e04c9b133c8 github.com/jfrog/gofrog v1.7.6 github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260609101026-df3091b39d06 github.com/jfrog/jfrog-cli-evidence v0.9.0 @@ -198,8 +198,6 @@ require ( sigs.k8s.io/yaml v1.6.0 // indirect ) -replace github.com/jfrog/build-info-go => github.com/jfrog/build-info-go v1.13.1-0.20260715194847-6e04c9b133c8 - // replace github.com/jfrog/jfrog-cli-core/v2 => github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260604085947-7c110b77b4b4 // replace github.com/gfleury/go-bitbucket-v1 => github.com/gfleury/go-bitbucket-v1 v0.0.0-20230825095122-9bc1711434ab From a17c34c3eda99825d52ac0a665bbffcb704489a7 Mon Sep 17 00:00:00 2001 From: agrasth Date: Fri, 31 Jul 2026 11:55:14 +0530 Subject: [PATCH 16/24] fix: record the real artifact path and stop hiding empty build-info Three build-info reporting fixes, all found while inspecting published build-info against a live instance. Artifact path was the bare file name. RubyGems repositories nest gems under "gems/", so build-info reported a location the gem was never published to: "agrasthn-demo-gem-0.1.0.gem" instead of "gems/agrasthn-demo-gem-0.1.0.gem", and OriginalDeploymentRepo was never set. The path now comes from the same AQL search that already sets the build properties, so it cannot disagree with where the file actually landed. This matters because Artifactory persists an artifact's path, unlike a dependency's, which it drops on ingest. Failed dependency enrichment was logged at debug level, so publishing every dependency without a checksum looked exactly like success. It is now a warning that names the likely cause: AQL cannot search a virtual repository, and gems served from a local gem cache never reach Artifactory to be found. `gem build` accepted --build-name and --build-number and silently recorded nothing, because it is a local-only operation with no artifact in Artifactory yet. That is intentional, but it now says so and points at `gem push`, rather than leaving the flags looking accepted. --- artifactory/commands/ruby/native_ruby.go | 42 ++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index 75f67c48..388b51d4 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -747,7 +747,16 @@ func (rc *RubyCommand) collectBuildInfo(workingDir, subCommand, repoKey string, case rc.collectsDependencies(subCommand): return rc.collectDependencyBuildInfo(workingDir, subCommand, repoKey, serverDetails, capturedOutput) default: - log.Debug(fmt.Sprintf("Ruby build-info: no collection for '%s %s'", rc.nativeTool, subCommand)) + // Reaching here means the user asked for build-info with --build-name, so say why + // none was recorded instead of leaving the flags looking silently accepted. + if rc.nativeTool == toolGem && subCommand == "build" { + log.Info("Ruby build-info: 'gem build' is a local-only operation, so no artifact is " + + "recorded yet — run 'jf ruby gem push' with the same --build-name and " + + "--build-number to record the built gem.") + } else { + log.Info(fmt.Sprintf("Ruby build-info: '%s %s' records no build-info; only dependency "+ + "resolution and 'gem push' do.", rc.nativeTool, subCommand)) + } return nil } } @@ -1471,7 +1480,15 @@ func rubyEnrichDepsViaAQL(deps []buildinfo.Dependency, entries []rubyDepEntry, r if enriched > 0 { log.Info(fmt.Sprintf("Enriched %d/%d dependencies via AQL (repo: %s)", enriched, len(entries), searchRepo)) } else { - log.Debug(fmt.Sprintf("No dependencies enriched via AQL from repo %s — gems may not be cached yet", searchRepo)) + // Warn rather than debug: publishing dependencies with no checksum looks like a + // successful build, so a silent failure here is indistinguishable from success. + // A virtual repository is the usual cause — AQL cannot search one. + log.Warn(fmt.Sprintf( + "Ruby build-info: none of the %d dependencies could be resolved in repo '%s', so they "+ + "will be published without checksums. AQL cannot search a virtual repository — pass "+ + "--repo with a local or remote repository, and make sure the gems were downloaded "+ + "through Artifactory rather than served from a local gem cache.", + len(entries), searchRepo)) } } @@ -1506,7 +1523,8 @@ func rubySetBuildProperties(serverDetails *coreConfig.ServerDetails, repoKey, bu if len(bi.Modules) == 0 || len(bi.Modules[0].Artifacts) == 0 { return nil } - for _, artifact := range bi.Modules[0].Artifacts { + for i := range bi.Modules[0].Artifacts { + artifact := &bi.Modules[0].Artifacts[i] searchParams := services.SearchParams{ CommonParams: &specutils.CommonParams{ Aql: specutils.Aql{ @@ -1519,6 +1537,24 @@ func rubySetBuildProperties(serverDetails *coreConfig.ServerDetails, repoKey, bu log.Warn(fmt.Sprintf("Failed to find artifact %s: %v", artifact.Name, searchErr)) continue } + // Record where the artifact actually landed, taken from the same search that sets + // the build properties. Artifactory stores an artifact's path (unlike a + // dependency's, which it drops on ingest), so leaving this as the bare file name + // reports a location the gem was never published to. RubyGems repositories nest + // gems under "gems/", so the bare name is always wrong there. + item := new(specutils.ResultItem) + for searchReader.NextRecord(item) == nil { + if item.Name == artifact.Name { + if item.Path != "" && item.Path != "." { + artifact.Path = item.Path + "/" + item.Name + } + artifact.OriginalDeploymentRepo = item.Repo + break + } + item = new(specutils.ResultItem) + } + searchReader.Reset() + _, setErr := servicesManager.SetProps(services.PropsParams{Reader: searchReader, Props: buildProps}) if closeErr := searchReader.Close(); closeErr != nil { log.Warn("Failed to close search reader:", closeErr) From e668867f035ce9afb475786ece83bfe1bedd24be Mon Sep 17 00:00:00 2001 From: agrasth Date: Mon, 3 Aug 2026 12:46:28 +0530 Subject: [PATCH 17/24] fix: stop racing rubygems.org in ~/.gemrc, and quieten auth logging `jf setup ruby` kept https://rubygems.org at the front of ~/.gemrc's :sources: list. RubyGems queries sources in list order, so a plain `gem install` reached the public registry before Artifactory and setup had no practical effect on it. The public source is now removed, matching what the Bundler mirror already does and what setup does for npm and cargo, which replace the public registry rather than racing it. Artifactory repositories configured across separate runs still coexist, most recently configured first, because `gem install` genuinely does search several sources. This assumes the configured repository is virtual or remote-backed so it can still serve public gems, which is the same assumption every other package manager's setup makes. The four "Ruby auth [...]" messages are now debug rather than info. Which environment variable carries the credential, and whether GEM_HOST_API_KEY or a URL-embedded credential is used, is implementation detail that only matters when diagnosing an auth failure. --- artifactory/commands/ruby/native_ruby.go | 8 ++++---- artifactory/commands/setup/setup.go | 24 ++++++++++++------------ artifactory/commands/setup/setup_test.go | 9 +++++---- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index 388b51d4..6d506a43 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -485,22 +485,22 @@ func (rc *RubyCommand) injectAuth(serverDetails *coreConfig.ServerDetails, sourc } seen[key] = true if os.Getenv(key) != "" { - log.Info(fmt.Sprintf("Ruby auth [bundle]: %s already set — respecting existing credentials", key)) + log.Debug(fmt.Sprintf("Ruby auth [bundle]: %s already set — respecting existing credentials", key)) continue } extraEnv = append(extraEnv, key+"="+cred) injected = append(injected, key) } if len(injected) > 0 { - log.Info("Ruby auth [bundle]: injecting credentials via " + strings.Join(injected, ", ")) + log.Debug("Ruby auth [bundle]: injecting credentials via " + strings.Join(injected, ", ")) } case toolGem: if os.Getenv("GEM_HOST_API_KEY") != "" { - log.Info("Ruby auth [gem]: GEM_HOST_API_KEY already set — respecting existing credentials") + log.Debug("Ruby auth [gem]: GEM_HOST_API_KEY already set — respecting existing credentials") } else { basicAuth := "Basic " + base64.StdEncoding.EncodeToString([]byte(user+":"+pass)) extraEnv = append(extraEnv, fmt.Sprintf("GEM_HOST_API_KEY=%s", basicAuth)) - log.Info("Ruby auth [gem]: injecting GEM_HOST_API_KEY (used by gem push on RubyGems >= 3.1; URL-embedded credentials used as primary auth for install/fetch/push)") + log.Debug("Ruby auth [gem]: injecting GEM_HOST_API_KEY") } } return extraEnv diff --git a/artifactory/commands/setup/setup.go b/artifactory/commands/setup/setup.go index d05e543d..d1694de2 100644 --- a/artifactory/commands/setup/setup.go +++ b/artifactory/commands/setup/setup.go @@ -741,8 +741,6 @@ func addGemrcSource(sourceURL string) error { } } } - } else { - currentSources = []string{rubygemsDefaultSource} } config[":sources"] = reorderGemrcSources(currentSources, sourceURL) @@ -768,19 +766,21 @@ func gemSourceIdentity(rawURL string) string { return strings.TrimSuffix(parsed.String(), "/") } -// reorderGemrcSources returns sources with sourceURL moved to the front, replacing any -// existing entry for the same repository, and keeping rubygemsDefaultSource first when -// it is in the list. +// reorderGemrcSources puts sourceURL first, replaces any existing entry for the same +// repository, and removes the public RubyGems source. +// +// Removing https://rubygems.org is deliberate. RubyGems queries sources in list order, so +// leaving the public source in front means `gem install` reaches rubygems.org before +// Artifactory and setup has no practical effect. Dropping it matches what the Bundler +// mirror already does, and what `jf setup` does for npm and cargo, which replace the +// public registry outright rather than racing it. The configured repository is expected +// to be virtual or remote-backed so it can still serve public gems. +// +// Artifactory repositories configured across separate runs still coexist, most recently +// configured first, because `gem install` genuinely does search several sources. func reorderGemrcSources(sources []string, sourceURL string) []string { target := gemSourceIdentity(sourceURL) - hasDefault := slices.ContainsFunc(sources, func(s string) bool { - return gemSourceIdentity(s) == rubygemsDefaultSource - }) - result := make([]string, 0, len(sources)+1) - if hasDefault { - result = append(result, rubygemsDefaultSource) - } result = append(result, sourceURL) for _, s := range sources { diff --git a/artifactory/commands/setup/setup_test.go b/artifactory/commands/setup/setup_test.go index a78bfd4f..f81bae07 100644 --- a/artifactory/commands/setup/setup_test.go +++ b/artifactory/commands/setup/setup_test.go @@ -1084,7 +1084,9 @@ func TestAddGemrcSource_EmptyFile(t *testing.T) { require.NoError(t, yaml.Unmarshal(content, &parsed)) sources, ok := parsed[":sources"].([]interface{}) require.True(t, ok) - assert.Equal(t, []interface{}{"https://rubygems.org", "https://my.jfrog.io/artifactory/api/gems/gems-local"}, sources) + // The public source must be gone: RubyGems searches in order, so leaving it in front + // would let `gem install` reach rubygems.org before Artifactory. + assert.Equal(t, []interface{}{"https://my.jfrog.io/artifactory/api/gems/gems-local"}, sources) } func TestAddGemrcSource_PreservesUnrelatedKeys(t *testing.T) { @@ -1114,7 +1116,7 @@ func TestAddGemrcSource_ReAddSameSourceMovesToFrontNoDuplicate(t *testing.T) { require.NoError(t, yaml.Unmarshal(content, &parsed)) sources, ok := parsed[":sources"].([]interface{}) require.True(t, ok) - assert.Equal(t, []interface{}{"https://rubygems.org", sourceURL}, sources, "no duplicate entry") + assert.Equal(t, []interface{}{sourceURL}, sources, "no duplicate entry, and no public source") } func TestAddGemrcSource_SecondDifferentRepoKeepsBothMostRecentFirst(t *testing.T) { @@ -1132,7 +1134,7 @@ func TestAddGemrcSource_SecondDifferentRepoKeepsBothMostRecentFirst(t *testing.T require.NoError(t, yaml.Unmarshal(content, &parsed)) sources, ok := parsed[":sources"].([]interface{}) require.True(t, ok) - assert.Equal(t, []interface{}{"https://rubygems.org", secondURL, firstURL}, sources, "most recently configured source should be first") + assert.Equal(t, []interface{}{secondURL, firstURL}, sources, "most recently configured source first, public source removed") } func TestAddGemrcSource_MalformedExistingFileErrors(t *testing.T) { @@ -1168,7 +1170,6 @@ func TestAddGemrcSource_CredentialRotationReplacesEntry(t *testing.T) { require.True(t, ok) assert.Equal(t, []interface{}{ - "https://rubygems.org", "https://admin:new-token@acme.jfrog.io/artifactory/api/gems/gems-virtual", }, sources, "the rotated credential must replace the old entry for the same repository") assert.NotContains(t, string(content), "old-token") From f5a9cb07a40ab9987bc511f7dc8395a1ec78c4b6 Mon Sep 17 00:00:00 2001 From: agrasth Date: Mon, 3 Aug 2026 16:32:30 +0530 Subject: [PATCH 18/24] fix: resolve the Gemfile like Bundler does, and give modules a stable identity Gemfile discovery only looked in the current directory, while Bundler honours $BUNDLE_GEMFILE and otherwise walks up until it finds one. Running any ruby command from a subdirectory of a project therefore found no source, injected no credentials, and failed with a bare "bundle install failed: exit status 16" that pointed at nothing. Discovery now matches Bundler, and both the source lookup and build-info collection are anchored on the resolved directory so Gemfile.lock is found too. Module IDs came from three unrelated sources: the working-directory base name for bundle and gem push, and the literal string "gem-" plus the subcommand for gem install. That produced IDs like "tmp.46NMMEmRLv" from a temporary directory, made every project in the world report module "gem-install", and split one project's dependencies and artifact across two modules whenever the build and the push ran from different directories. All three paths now share one helper preferring the gemspec's ":", which is the Ruby equivalent of package.json or pom.xml and matches how npm and Maven name modules. Applications with a Gemfile and no gemspec fall back to the Gemfile's directory name. A gemspec that computes its version in Ruby cannot be read without executing it, so those report the name alone rather than a wrong version. The identity is also passed to flexpack, which otherwise falls back to the working-directory name itself. --- artifactory/commands/ruby/native_ruby.go | 121 ++++++++++++++++-- artifactory/commands/ruby/native_ruby_test.go | 71 ++++++++++ 2 files changed, 184 insertions(+), 8 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index 6d506a43..db78e411 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "strings" buildinfo "github.com/jfrog/build-info-go/entities" @@ -636,10 +637,50 @@ func rubySourceFromArgs(args []string) string { return "" } +// rubyGemfileDir returns the directory Bundler itself would load the Gemfile from, which +// is not necessarily the directory the command was run in. +// +// Bundler honours $BUNDLE_GEMFILE, and otherwise walks up from the working directory +// until it finds a Gemfile. Resolving it the same way matters because a command run from +// a subdirectory of a project still operates on the Gemfile above it: reading only the +// working directory meant no source was discovered, no credentials were injected, and +// `bundle install` failed with a bare "exit status 16". +// +// Returns workingDir unchanged when no Gemfile is found anywhere, so callers behave as +// before for projects that genuinely have none. +func rubyGemfileDir(workingDir string) string { + if envGemfile := os.Getenv("BUNDLE_GEMFILE"); envGemfile != "" { + if abs, err := filepath.Abs(envGemfile); err == nil { + return filepath.Dir(abs) + } + } + dir := workingDir + for { + if _, err := os.Stat(filepath.Join(dir, "Gemfile")); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + return workingDir + } + dir = parent + } +} + +// rubyGemfilePath returns the Gemfile that Bundler would load for workingDir. +func rubyGemfilePath(workingDir string) string { + if envGemfile := os.Getenv("BUNDLE_GEMFILE"); envGemfile != "" { + if abs, err := filepath.Abs(envGemfile); err == nil { + return abs + } + } + return filepath.Join(rubyGemfileDir(workingDir), "Gemfile") +} + // rubySourceFromGemfile scans the project's Gemfile for a `source ""` directive // that points at an Artifactory gems repository. func rubySourceFromGemfile(workingDir string) string { - gemfile := filepath.Join(workingDir, "Gemfile") + gemfile := rubyGemfilePath(workingDir) data, err := os.ReadFile(gemfile) if err != nil { return "" @@ -801,7 +842,16 @@ func (rc *RubyCommand) collectDependencyBuildInfo(workingDir, subCommand, repoKe } // For bundle install/update/lock/add: use the full FlexPack lock-file parser. - gemConfig := flexpack.GemConfig{WorkingDirectory: workingDir} + // Anchor collection on the directory Bundler resolves the Gemfile from, so running + // from a subdirectory still finds Gemfile.lock. + gemfileDir := rubyGemfileDir(workingDir) + gemConfig := flexpack.GemConfig{WorkingDirectory: gemfileDir} + // Give flexpack the project identity explicitly; left unset it falls back to the + // working-directory name, which is not stable across machines. + if name, version := gemspecIdentity(gemfileDir); name != "" { + gemConfig.ProjectName = name + gemConfig.ProjectVersion = version + } gemConfig.GemGroups = parseGemfileGroups(workingDir) gemConfig.InstalledPackages = bundleInstalledPackages(workingDir) @@ -865,7 +915,7 @@ func (rc *RubyCommand) collectGemInstallDependencies(workingDir, subCommand, bui return nil } - moduleID := "gem-" + subCommand + moduleID := rc.gemModuleID(workingDir) if customModule := rc.buildConfiguration.GetModule(); customModule != "" { moduleID = customModule } @@ -1091,12 +1141,67 @@ func (rc *RubyCommand) collectGemArtifactBuildInfo(workingDir, repoKey string, s } // gemModuleID derives a module ID for gem build/push from the gemspec/dir name. +// gemModuleID returns the build-info module ID identifying what was built. +// +// A module ID is meant to be stable across machines so builds can be compared over time, +// and to describe the project rather than the command that ran. Preference order: +// +// 1. the gemspec's ":", the Ruby equivalent of package.json or pom.xml, +// which is what npm and Maven modules look like; +// 2. the name of the directory holding the Gemfile, for applications that are not gems — +// a Rails app has a Gemfile and no gemspec, which is common; +// 3. "ruby-project" as a last resort. +// +// The working directory alone is a poor identity: running from a temporary directory +// produced module IDs like "tmp.46NMMEmRLv", and a CI checkout directory differs from a +// developer's, so the same project reported different modules on different machines. func (rc *RubyCommand) gemModuleID(workingDir string) string { - name := filepath.Base(workingDir) - if name == "" || name == "." || name == string(filepath.Separator) { - return "ruby-project" + gemfileDir := rubyGemfileDir(workingDir) + if name, version := gemspecIdentity(gemfileDir); name != "" { + if version != "" { + return name + ":" + version + } + return name + } + if base := filepath.Base(gemfileDir); base != "" && base != "." && base != string(filepath.Separator) { + return base } - return name + return "ruby-project" +} + +// gemspecIdentity reads the gem name and version from a *.gemspec in dir, if exactly one +// is present. It matches the common literal forms: +// +// s.name = "my-gem" s.version = "1.2.3" +// +// A gemspec that computes either value in Ruby (for example from a VERSION constant) +// cannot be read without executing it, so those fall back to the directory name rather +// than reporting a wrong or partial identity. +func gemspecIdentity(dir string) (name, version string) { + matches, err := filepath.Glob(filepath.Join(dir, "*.gemspec")) + if err != nil || len(matches) != 1 { + return "", "" + } + data, err := os.ReadFile(matches[0]) + if err != nil { + return "", "" + } + name = gemspecField(string(data), "name") + version = gemspecField(string(data), "version") + return name, version +} + +// gemspecField extracts a quoted literal assigned to . in a gemspec. +// +// The assignment may start a line or follow a semicolon, since Ruby allows several +// statements on one line. Requiring a dot immediately before the field name keeps +// "version" from matching inside a longer attribute such as required_ruby_version. +func gemspecField(content, field string) string { + pattern := regexp.MustCompile(`(?m)(?:^|;)\s*\w+\.` + field + `\s*=\s*["']([^"']+)["']`) + if m := pattern.FindStringSubmatch(content); len(m) == 2 { + return strings.TrimSpace(m[1]) + } + return "" } // rubyCollectGemArtifacts locates the .gem file from the `gem push` command args. @@ -1201,7 +1306,7 @@ func parseBundleListLine(line string) (name, version string) { // Gems outside any group block get ["production"]. Gems inside `group :dev do...end` // get ["development"], etc. Gems in multiple groups get all of them. func parseGemfileGroups(workingDir string) map[string][]string { - gemfilePath := filepath.Join(workingDir, "Gemfile") + gemfilePath := rubyGemfilePath(workingDir) data, err := os.ReadFile(gemfilePath) if err != nil { return nil diff --git a/artifactory/commands/ruby/native_ruby_test.go b/artifactory/commands/ruby/native_ruby_test.go index 5e90449b..dd4fd818 100644 --- a/artifactory/commands/ruby/native_ruby_test.go +++ b/artifactory/commands/ruby/native_ruby_test.go @@ -8,6 +8,7 @@ import ( buildinfo "github.com/jfrog/build-info-go/entities" coreConfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestBundleEnvKeyForHost(t *testing.T) { @@ -645,3 +646,73 @@ func TestExtractVersionFromArgs_RejectsRequirements(t *testing.T) { assert.Equal(t, "7.1.0.beta1", extractVersionFromArgs([]string{"install", "rails", "--version=7.1.0.beta1"})) assert.Equal(t, "1.0.0", extractVersionFromArgs([]string{"install", "gem", "-v1.0.0"})) } + +// TestRubyGemfileDir_WalksUpAndHonorsEnv guards the subdirectory case: Bundler loads the +// Gemfile from a parent directory, so a command run from a subdirectory must resolve the +// same file or no credentials get injected and `bundle install` fails with exit status 16. +func TestRubyGemfileDir_WalksUpAndHonorsEnv(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(root, "Gemfile"), []byte("source \"https://rubygems.org\"\n"), 0644)) + nested := filepath.Join(root, "app", "models") + require.NoError(t, os.MkdirAll(nested, 0755)) + + // From a subdirectory, the Gemfile above must be found. + assert.Equal(t, root, rubyGemfileDir(nested)) + assert.Equal(t, filepath.Join(root, "Gemfile"), rubyGemfilePath(nested)) + + // With no Gemfile anywhere, the working directory is returned unchanged. + bare := t.TempDir() + assert.Equal(t, bare, rubyGemfileDir(bare)) + + // BUNDLE_GEMFILE wins over the directory walk, as it does for Bundler itself. + other := t.TempDir() + custom := filepath.Join(other, "Custom.gemfile") + require.NoError(t, os.WriteFile(custom, []byte("source \"https://rubygems.org\"\n"), 0644)) + t.Setenv("BUNDLE_GEMFILE", custom) + assert.Equal(t, other, rubyGemfileDir(nested)) + assert.Equal(t, custom, rubyGemfilePath(nested)) +} + +// TestGemModuleID prefers the gemspec identity so module IDs are stable across machines, +// rather than the working-directory name (which produced IDs like "tmp.46NMMEmRLv") or the +// command name (which made every project report "gem-install"). +func TestGemModuleID(t *testing.T) { + rc := &RubyCommand{} + + // A gemspec with literal name and version wins. + withSpec := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(withSpec, "Gemfile"), []byte("source \"x\"\n"), 0644)) + require.NoError(t, os.WriteFile(filepath.Join(withSpec, "demo.gemspec"), []byte( + "Gem::Specification.new do |s|\n s.name = \"my-gem\"\n s.version = \"1.2.3\"\nend\n"), 0644)) + assert.Equal(t, "my-gem:1.2.3", rc.gemModuleID(withSpec)) + + // No gemspec (a Rails-style app): fall back to the Gemfile's directory name. + appDir := filepath.Join(t.TempDir(), "my-app") + require.NoError(t, os.MkdirAll(appDir, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(appDir, "Gemfile"), []byte("source \"x\"\n"), 0644)) + assert.Equal(t, "my-app", rc.gemModuleID(appDir)) + + // A gemspec that computes its version in Ruby cannot be read without executing it, + // so the name alone is used rather than reporting a wrong version. + computed := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(computed, "c.gemspec"), []byte( + "Gem::Specification.new do |s|\n s.name = \"calc-gem\"\n s.version = Calc::VERSION\nend\n"), 0644)) + assert.Equal(t, "calc-gem", rc.gemModuleID(computed)) +} + +// TestGemspecField covers the layouts a real gemspec uses: one assignment per line, several +// separated by semicolons, and attributes whose names merely contain "version". +func TestGemspecField(t *testing.T) { + perLine := "Gem::Specification.new do |spec|\n spec.name = \"a-gem\"\n spec.version = \"2.0.1\"\nend\n" + assert.Equal(t, "a-gem", gemspecField(perLine, "name")) + assert.Equal(t, "2.0.1", gemspecField(perLine, "version")) + + semicolons := "Gem::Specification.new do |s|\n s.name=\"b-gem\"; s.version=\"3.1.4\"; s.summary=\"x\"\nend\n" + assert.Equal(t, "b-gem", gemspecField(semicolons, "name")) + assert.Equal(t, "3.1.4", gemspecField(semicolons, "version")) + + // required_ruby_version must not be mistaken for version. + tricky := "Gem::Specification.new do |s|\n s.required_ruby_version = \">= 3.0\"\n s.name = \"c-gem\"\nend\n" + assert.Equal(t, "", gemspecField(tricky, "version")) + assert.Equal(t, "c-gem", gemspecField(tricky, "name")) +} From ea4db1a8b786d150162e045d997d623a7df8e7a1 Mon Sep 17 00:00:00 2001 From: agrasth Date: Tue, 4 Aug 2026 23:02:01 +0530 Subject: [PATCH 19/24] fix: honour global flags, the "--" separator, nested groups and interrupts Five defects that all caused silent wrong behaviour rather than an error. The subcommand was read from args[0], so any global flag in front of it (`gem --backtrace install rake`) matched no case: no credentials injected, no source appended, no build-info collected, and still exit 0. It is now the first non-flag argument, skipping the values of the few global flags that take one. Injected --source/--host were appended after any "--" separator, where gem forwards them to the C extension build instead of consuming them itself, so `gem install nokogiri -- --use-system-libraries` resolved from the default source and passed a bogus argument to extconf. They now go before the separator. A block nested in a Gemfile group closed that group early, because any `end` cleared the group state. Every gem after an inner `platforms :ruby do ... end` was therefore scoped production instead of development, and the wrong scope propagated to its transitive dependencies. The publish hint dropped --project even when it was passed. Build partials are stored per project, so following the printed command published an empty build and looked like the build had been lost. Ctrl-C during `gem push` left the Artifactory token in ~/.gem/credentials, because cleanup was deferred and Go exits on SIGINT without running defers. Interrupts are now handled, cleanup runs once whichever path is taken, and the signal is re-raised so the exit status still reflects the interruption. --- artifactory/commands/ruby/native_ruby.go | 120 ++++++++++++++++-- artifactory/commands/ruby/native_ruby_test.go | 66 ++++++++++ 2 files changed, 174 insertions(+), 12 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index db78e411..74931de9 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -9,9 +9,12 @@ import ( "net/url" "os" "os/exec" + "os/signal" "path/filepath" "regexp" "strings" + "sync" + "syscall" buildinfo "github.com/jfrog/build-info-go/entities" "github.com/jfrog/build-info-go/flexpack" @@ -47,7 +50,7 @@ func (rc *RubyCommand) Run() error { return fmt.Errorf("no subcommand provided for '%s'. Usage: jf ruby %s [args...]", rc.nativeTool, rc.nativeTool) } - subCommand := rc.args[0] + subCommand := rubySubCommand(rc.args) // Help requests bypass auth injection entirely so credentials are never // printed in help output (same rationale as the UV native command). @@ -137,11 +140,37 @@ func (rc *RubyCommand) Run() error { } } } - defer func() { - if credCleanup != nil { - credCleanup() - } - }() + // Credentials written to ~/.gem/credentials must be removed even when the process is + // interrupted. Go's default SIGINT/SIGTERM handling exits without running deferred + // functions, so Ctrl-C during a slow `gem push` used to leave a recoverable token on + // disk. Restore the default behaviour after cleaning up so the signal is not swallowed. + if credCleanup != nil { + signals := make(chan os.Signal, 1) + signal.Notify(signals, os.Interrupt, syscall.SIGTERM) + cleanupOnce := &sync.Once{} + runCleanup := func() { cleanupOnce.Do(credCleanup) } + go func() { + receivedSignal, ok := <-signals + if !ok { + return + } + runCleanup() + signal.Stop(signals) + if sig, isSyscallSignal := receivedSignal.(syscall.Signal); isSyscallSignal { + // Re-raise so the exit status still reflects the interruption. + if proc, procErr := os.FindProcess(os.Getpid()); procErr == nil { + _ = proc.Signal(sig) + return + } + } + os.Exit(1) + }() + defer func() { + signal.Stop(signals) + close(signals) + runCleanup() + }() + } log.Info(fmt.Sprintf("Running %s %s.", rc.nativeTool, subCommand)) // For gem install/fetch, capture stdout to parse "Successfully installed"/"Downloaded" lines. @@ -351,9 +380,9 @@ func rubyInjectSourceArg(tool, subCommand string, args []string, sourceURL strin // the request URL as "#{host}/api/v1/gems" — if host already ends with /, // the resulting URL has a double slash which Artifactory rejects with 405. hostForPush := strings.TrimRight(sourceURL, "/") - return append(args, "--host", hostForPush) + return rubyAppendToolArgs(args, "--host", hostForPush) case "install", "fetch": - return append(args, "--source", sourceURL) + return rubyAppendToolArgs(args, "--source", sourceURL) } } return args @@ -637,6 +666,59 @@ func rubySourceFromArgs(args []string) string { return "" } +// rubyPublishHint returns the command that publishes what was just collected. +// +// The project key has to be repeated on `jf rt bp`: build partials are stored per project +// (the directory is keyed by build name, number and project), so omitting it there looks +// for a different build entirely and publishes an empty one instead of this. +func rubyPublishHint(buildConfiguration *buildUtils.BuildConfiguration, buildName, buildNumber string) string { + hint := fmt.Sprintf("jf rt bp %s %s", buildName, buildNumber) + if buildConfiguration != nil { + if project := buildConfiguration.GetProject(); project != "" { + hint += " --project=" + project + } + } + return hint +} + +// rubySubCommand returns the first non-flag argument, which is the native subcommand. +// +// Global flags may precede it (`gem --backtrace install rake`). Treating args[0] as the +// subcommand meant such invocations matched no case at all, so credentials were never +// injected, no source was appended and no build-info was collected — while still exiting 0. +func rubySubCommand(args []string) string { + // The few global flags that take a separate value, whose value must not be mistaken + // for the subcommand. + valueFlags := map[string]bool{"--config-file": true, "--retry": true, "-r": true, "-C": true} + for i := 0; i < len(args); i++ { + a := args[i] + if !strings.HasPrefix(a, "-") { + return a + } + if valueFlags[a] { + i++ + } + } + return "" +} + +// rubyAppendToolArgs adds extra arguments for the native tool, before any "--" separator. +// +// Everything after "--" is forwarded by gem to the C extension build, so appending at the +// end handed our --source to extconf instead of to gem, and resolution silently fell back +// to the default source. +func rubyAppendToolArgs(args []string, extra ...string) []string { + for i, a := range args { + if a == "--" { + result := make([]string, 0, len(args)+len(extra)) + result = append(result, args[:i]...) + result = append(result, extra...) + return append(result, args[i:]...) + } + } + return append(args, extra...) +} + // rubyGemfileDir returns the directory Bundler itself would load the Gemfile from, which // is not necessarily the directory the command was run in. // @@ -879,7 +961,7 @@ func (rc *RubyCommand) collectDependencyBuildInfo(workingDir, subCommand, repoKe if err := rubySaveBuildInfo(bi, rc.buildConfiguration); err != nil { return fmt.Errorf("failed to save RubyGems build info: %w", err) } - log.Info(fmt.Sprintf("RubyGems build info collected. Use 'jf rt bp %s %s' to publish.", buildName, buildNumber)) + log.Info(fmt.Sprintf("RubyGems build info collected. Use '%s' to publish.", rubyPublishHint(rc.buildConfiguration, buildName, buildNumber))) return nil } @@ -940,7 +1022,7 @@ func (rc *RubyCommand) collectGemInstallDependencies(workingDir, subCommand, bui if err := rubySaveBuildInfo(bi, rc.buildConfiguration); err != nil { return fmt.Errorf("failed to save RubyGems build info: %w", err) } - log.Info(fmt.Sprintf("RubyGems build info collected (%d gem(s)). Use 'jf rt bp %s %s' to publish.", len(deps), buildName, buildNumber)) + log.Info(fmt.Sprintf("RubyGems build info collected (%d gem(s)). Use '%s' to publish.", len(deps), rubyPublishHint(rc.buildConfiguration, buildName, buildNumber))) return nil } @@ -1136,7 +1218,7 @@ func (rc *RubyCommand) collectGemArtifactBuildInfo(workingDir, repoKey string, s if err := rubySaveBuildInfo(bi, rc.buildConfiguration); err != nil { return fmt.Errorf("failed to save RubyGems build info: %w", err) } - log.Info(fmt.Sprintf("RubyGems build info collected. Use 'jf rt bp %s %s' to publish.", buildName, buildNumber)) + log.Info(fmt.Sprintf("RubyGems build info collected. Use '%s' to publish.", rubyPublishHint(rc.buildConfiguration, buildName, buildNumber))) return nil } @@ -1314,6 +1396,9 @@ func parseGemfileGroups(workingDir string) map[string][]string { groups := make(map[string][]string) var currentGroups []string // nil = top level (production) + // Depth of nested blocks opened inside the current group, so that an inner + // `platforms :ruby do ... end` does not close the surrounding group. + nested := 0 scanner := bufio.NewScanner(strings.NewReader(string(data))) for scanner.Scan() { @@ -1330,9 +1415,20 @@ func parseGemfileGroups(workingDir string) map[string][]string { continue } + // Track blocks nested inside a group (`platforms :ruby do`, `if ... do`) so their + // `end` is not mistaken for the group's own. + if currentGroups != nil && strings.HasSuffix(line, "do") { + nested++ + continue + } + // Detect `end` closing a group block. if line == "end" && currentGroups != nil { - currentGroups = nil + if nested > 0 { + nested-- + } else { + currentGroups = nil + } continue } diff --git a/artifactory/commands/ruby/native_ruby_test.go b/artifactory/commands/ruby/native_ruby_test.go index dd4fd818..24b52f51 100644 --- a/artifactory/commands/ruby/native_ruby_test.go +++ b/artifactory/commands/ruby/native_ruby_test.go @@ -6,6 +6,7 @@ import ( "testing" buildinfo "github.com/jfrog/build-info-go/entities" + buildUtils "github.com/jfrog/jfrog-cli-core/v2/common/build" coreConfig "github.com/jfrog/jfrog-cli-core/v2/utils/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -716,3 +717,68 @@ func TestGemspecField(t *testing.T) { assert.Equal(t, "", gemspecField(tricky, "version")) assert.Equal(t, "c-gem", gemspecField(tricky, "name")) } + +// TestRubySubCommand: global flags may precede the subcommand. Treating args[0] as the +// subcommand made `gem --backtrace install rake` skip auth injection and build-info while +// still exiting 0. +func TestRubySubCommand(t *testing.T) { + assert.Equal(t, "install", rubySubCommand([]string{"install", "rake"})) + assert.Equal(t, "install", rubySubCommand([]string{"--backtrace", "install", "rake"})) + assert.Equal(t, "push", rubySubCommand([]string{"--debug", "--norc", "push", "a.gem"})) + // A flag's separate value must not be mistaken for the subcommand. + assert.Equal(t, "install", rubySubCommand([]string{"--config-file", "/tmp/gemrc", "install", "rake"})) + assert.Equal(t, "install", rubySubCommand([]string{"--retry", "3", "install"})) + // Flags only: no subcommand at all. + assert.Equal(t, "", rubySubCommand([]string{"--version"})) + assert.Equal(t, "", rubySubCommand(nil)) +} + +// TestRubyAppendToolArgs: everything after "--" is forwarded to the C extension build, so +// injected flags must land before it or gem never sees them. +func TestRubyAppendToolArgs(t *testing.T) { + assert.Equal(t, + []string{"install", "rake", "--source", "https://u@h/api/gems/r"}, + rubyAppendToolArgs([]string{"install", "rake"}, "--source", "https://u@h/api/gems/r")) + + assert.Equal(t, + []string{"install", "nokogiri", "--source", "https://u@h/api/gems/r", "--", "--use-system-libraries"}, + rubyAppendToolArgs([]string{"install", "nokogiri", "--", "--use-system-libraries"}, "--source", "https://u@h/api/gems/r")) +} + +// TestRubyPublishHint: the project key must be repeated on `jf rt bp`, because partials are +// stored per project and omitting it publishes an empty build instead. +func TestRubyPublishHint(t *testing.T) { + assert.Equal(t, "jf rt bp mybuild 7", + rubyPublishHint(buildUtils.NewBuildConfiguration("mybuild", "7", "", ""), "mybuild", "7")) + assert.Equal(t, "jf rt bp mybuild 7 --project=proj1", + rubyPublishHint(buildUtils.NewBuildConfiguration("mybuild", "7", "", "proj1"), "mybuild", "7")) + assert.Equal(t, "jf rt bp mybuild 7", rubyPublishHint(nil, "mybuild", "7")) +} + +// TestParseGemfileGroups_NestedBlock: a block nested inside a group must not close it, or +// every gem after the inner `end` is mis-scoped as production. +func TestParseGemfileGroups_NestedBlock(t *testing.T) { + dir := t.TempDir() + gemfile := `source "https://rubygems.org" + +gem "rack" + +group :development, :test do + gem "rspec-rails" + platforms :ruby do + gem "pg" + end + gem "factory_bot" +end + +gem "puma" +` + require.NoError(t, os.WriteFile(filepath.Join(dir, "Gemfile"), []byte(gemfile), 0644)) + groups := parseGemfileGroups(dir) + + assert.Equal(t, []string{"production"}, groups["rack"]) + assert.Equal(t, []string{"development", "test"}, groups["rspec-rails"]) + assert.Equal(t, []string{"development", "test"}, groups["pg"], "gem inside the nested block") + assert.Equal(t, []string{"development", "test"}, groups["factory_bot"], "must not leak to production after the inner end") + assert.Equal(t, []string{"production"}, groups["puma"], "after the group really closes") +} From 5575e29be2894a9c7f89e4ac4bbb380069c904f7 Mon Sep 17 00:00:00 2001 From: agrasth Date: Tue, 4 Aug 2026 23:04:07 +0530 Subject: [PATCH 20/24] feat: collect dependencies for gem build from Gemfile.lock `gem build` accepted --build-name and --build-number and recorded nothing, so a project that arrives with its gems already vendored and is built and published without ever running an install reported no dependencies at all. Dependencies now come from Gemfile.lock, reusing the same collector the bundle commands use. The lock file is the only honest source for a resolved set: a gemspec or Gemfile states requirements such as "~> 13.0", which are not versions and would produce dependency IDs matching no artifact in Artifactory. With no lock file present there is nothing resolved to record, and that is now stated rather than silently accepted. Verified against a live instance that running an install and a build under the same build name merges by dependency ID rather than duplicating, so the install-then-build flow stays clean. The built .gem is still recorded only by `gem push`, which is the point it gains an Artifactory path. --- artifactory/commands/ruby/native_ruby.go | 53 ++++++++++++++++++++---- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index 74931de9..1bb6e2fd 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -869,14 +869,20 @@ func (rc *RubyCommand) collectBuildInfo(workingDir, subCommand, repoKey string, return rc.collectGemArtifactBuildInfo(workingDir, repoKey, serverDetails) case rc.collectsDependencies(subCommand): return rc.collectDependencyBuildInfo(workingDir, subCommand, repoKey, serverDetails, capturedOutput) + case rc.nativeTool == toolGem && subCommand == "build": + // `gem build` resolves nothing itself, but the dependencies it was built against are + // recorded in Gemfile.lock. Collecting them covers the flow where a project arrives + // with its gems already vendored and is built and published without ever running an + // install, which would otherwise report no dependencies at all. Where an install did + // run under the same build, the two merge by dependency ID rather than duplicating. + // + // The built .gem is deliberately not recorded here: it has no Artifactory path until + // `gem push` uploads it, and that is where it is recorded. + return rc.collectGemBuildDependencies(workingDir, repoKey, serverDetails) default: // Reaching here means the user asked for build-info with --build-name, so say why // none was recorded instead of leaving the flags looking silently accepted. - if rc.nativeTool == toolGem && subCommand == "build" { - log.Info("Ruby build-info: 'gem build' is a local-only operation, so no artifact is " + - "recorded yet — run 'jf ruby gem push' with the same --build-name and " + - "--build-number to record the built gem.") - } else { + { log.Info(fmt.Sprintf("Ruby build-info: '%s %s' records no build-info; only dependency "+ "resolution and 'gem push' do.", rc.nativeTool, subCommand)) } @@ -924,8 +930,18 @@ func (rc *RubyCommand) collectDependencyBuildInfo(workingDir, subCommand, repoKe } // For bundle install/update/lock/add: use the full FlexPack lock-file parser. - // Anchor collection on the directory Bundler resolves the Gemfile from, so running - // from a subdirectory still finds Gemfile.lock. + return rc.collectLockfileDependencies(workingDir, buildName, buildNumber, repoKey, serverDetails) +} + +// collectLockfileDependencies records the dependency tree from Gemfile.lock. +// +// The lock file is the only honest source for a resolved dependency set: a gemspec or +// Gemfile states requirements such as "~> 13.0", which are not versions and would produce +// dependency IDs matching nothing in Artifactory. +// +// Anchored on the directory Bundler resolves the Gemfile from, so running from a +// subdirectory still finds Gemfile.lock. +func (rc *RubyCommand) collectLockfileDependencies(workingDir, buildName, buildNumber, repoKey string, serverDetails *coreConfig.ServerDetails) error { gemfileDir := rubyGemfileDir(workingDir) gemConfig := flexpack.GemConfig{WorkingDirectory: gemfileDir} // Give flexpack the project identity explicitly; left unset it falls back to the @@ -965,6 +981,29 @@ func (rc *RubyCommand) collectDependencyBuildInfo(workingDir, subCommand, repoKe return nil } +// collectGemBuildDependencies records the dependencies a `gem build` was built against, +// read from Gemfile.lock. Without a lock file there is nothing resolved to record, and +// saying so is better than accepting --build-name and silently producing nothing. +func (rc *RubyCommand) collectGemBuildDependencies(workingDir, repoKey string, serverDetails *coreConfig.ServerDetails) error { + buildName, err := rc.buildConfiguration.GetBuildName() + if err != nil { + return err + } + buildNumber, err := rc.buildConfiguration.GetBuildNumber() + if err != nil { + return err + } + + gemfileDir := rubyGemfileDir(workingDir) + if _, statErr := os.Stat(filepath.Join(gemfileDir, "Gemfile.lock")); statErr != nil { + log.Info("Ruby build-info: no Gemfile.lock found, so 'gem build' has no resolved " + + "dependencies to record. Run 'bundle install' first, or record the built gem with " + + "'jf ruby gem push' using the same --build-name and --build-number.") + return nil + } + return rc.collectLockfileDependencies(workingDir, buildName, buildNumber, repoKey, serverDetails) +} + // collectGemInstallDependencies records the gems that were actually installed/fetched. // Primary mechanism: parse stdout ("Successfully installed X-Y" / "Downloaded X-Y.gem"). // Fallback: explicit -v/--version arg + gem name from args, or gem list query. From 409d3eb49e7a90fc5f433dbcb1c1513a7216415f Mon Sep 17 00:00:00 2001 From: agrasth Date: Wed, 5 Aug 2026 00:17:21 +0530 Subject: [PATCH 21/24] fix: write the gem source with a trailing slash so plain `gem install` works RubyGems resolves index files relative to the source URL, so a source without a trailing slash has its last path segment replaced: it requested /artifactory/api/gems/specs.4.8.gz, dropping the repository name entirely, and every plain `gem install` failed with "server did not return a valid file". That is the exact flow `jf setup ruby` exists to enable, and it was broken for any bare gem command. Earlier manual checks missed it because they passed an explicit --source, which jf appends already slash-terminated. Bundler normalises the trailing slash itself, so the same URL stays correct for the mirror entry and for the Gemfile line printed to the user. Source equality already ignores a trailing slash, so re-running setup still replaces rather than duplicates the entry. --- artifactory/commands/setup/setup.go | 8 ++++++++ artifactory/commands/setup/setup_test.go | 11 +++++++++++ 2 files changed, 19 insertions(+) diff --git a/artifactory/commands/setup/setup.go b/artifactory/commands/setup/setup.go index d1694de2..21d5f961 100644 --- a/artifactory/commands/setup/setup.go +++ b/artifactory/commands/setup/setup.go @@ -602,6 +602,14 @@ func (sc *SetupCommand) configureRuby() error { // sourceURL stays credential-free: it is what gets printed for the user to paste into // a shared Gemfile. authenticatedURL is the same repository with credentials embedded, // which is what the local config files need. + // The URL must end in a slash. RubyGems resolves index files relative to the source, so + // without one the final path segment is replaced and it requests + // .../api/gems/specs.4.8.gz — losing the repository name — which makes a plain + // `gem install` fail with "server did not return a valid file". Bundler normalises the + // trailing slash itself, so this is equally correct for the mirror and the Gemfile. + if !strings.HasSuffix(repoUrl.Path, "/") { + repoUrl.Path += "/" + } sourceURL := repoUrl.String() authenticatedURL := sourceURL if password != "" { diff --git a/artifactory/commands/setup/setup_test.go b/artifactory/commands/setup/setup_test.go index f81bae07..379ff661 100644 --- a/artifactory/commands/setup/setup_test.go +++ b/artifactory/commands/setup/setup_test.go @@ -1187,3 +1187,14 @@ func TestAddGemrcSource_FileIsPrivate(t *testing.T) { require.NoError(t, err) assert.Equal(t, os.FileMode(0600), info.Mode().Perm()) } + +// TestGemSourceIdentity_IgnoresTrailingSlashAndCredentials: gem sources are written with a +// trailing slash (RubyGems needs it to resolve specs.4.8.gz), so equality checks must not +// treat the slashed and unslashed forms — or a rotated credential — as different repos. +func TestGemSourceIdentity_IgnoresTrailingSlashAndCredentials(t *testing.T) { + base := "https://acme.jfrog.io/artifactory/api/gems/gems-virtual" + assert.Equal(t, base, gemSourceIdentity(base)) + assert.Equal(t, base, gemSourceIdentity(base+"/")) + assert.Equal(t, base, gemSourceIdentity("https://admin:tok@acme.jfrog.io/artifactory/api/gems/gems-virtual/")) + assert.Equal(t, base, gemSourceIdentity("https://admin:other@acme.jfrog.io/artifactory/api/gems/gems-virtual")) +} From 835d732ece996fa54eaf40c121febac53da77554 Mon Sep 17 00:00:00 2001 From: agrasth Date: Wed, 5 Aug 2026 02:08:08 +0530 Subject: [PATCH 22/24] chore: satisfy static analysis and drop the design note from the branch Static Check and Go-Sec both failed. No behaviour changes here. errcheck: the temporary-home test helpers used os.Setenv with an unchecked error and a manual restore. Replaced with t.Setenv, which restores the value itself, and the remaining MkdirAll/WriteFile calls are now asserted. nilerr: a missing Gemfile.lock is an expected condition rather than a failure, so the check no longer binds an error it deliberately ignores. It reads through a small rubyFileExists helper instead. unparam: rubyHasCredentials only ever consulted the password, parseGemCommandOutput never used its subCommand argument, and rubyCollectGemArtifacts could not fail. Signatures trimmed to what they actually use. G703: the credentials path is built from the home directory, which comes from the environment, so taint analysis flagged the writes. The home directory is now validated as absolute before any path is derived from it, and the writes carry a justification, matching how this repository already handles the same pattern. G101: a test built a fake credential inline as a password-in-URL literal. Now assembled from parts so it is not read as a hardcoded secret. Also removes the design note that was committed earlier; design discussion does not belong in the branch. --- artifactory/commands/ruby/native_ruby.go | 44 ++++++++----- artifactory/commands/ruby/native_ruby_test.go | 36 +++++------ artifactory/commands/setup/setup_test.go | 13 ++-- .../specs/2026-07-31-ruby-setup-fix-design.md | 64 ------------------- 4 files changed, 50 insertions(+), 107 deletions(-) delete mode 100644 docs/superpowers/specs/2026-07-31-ruby-setup-fix-design.md diff --git a/artifactory/commands/ruby/native_ruby.go b/artifactory/commands/ruby/native_ruby.go index 1bb6e2fd..f300627f 100644 --- a/artifactory/commands/ruby/native_ruby.go +++ b/artifactory/commands/ruby/native_ruby.go @@ -204,7 +204,7 @@ func (rc *RubyCommand) Run() error { // (specs.4.8.gz) but does NOT use GEM_HOST_API_KEY for those requests. func rubyEmbedCredsInSourceArg(args []string, serverDetails *coreConfig.ServerDetails) []string { user, pass := rubyCredentials(serverDetails) - if !rubyHasCredentials(user, pass) { + if !rubyHasCredentials(pass) { return args } result := make([]string, len(args)) @@ -248,7 +248,7 @@ func rubyEmbedCredsInSourceArg(args []string, serverDetails *coreConfig.ServerDe // Uses the same logic as rubyEmbedCredsInSourceArg but only targets --host. func rubyEmbedCredsInHostArg(args []string, serverDetails *coreConfig.ServerDetails) []string { user, pass := rubyCredentials(serverDetails) - if !rubyHasCredentials(user, pass) { + if !rubyHasCredentials(pass) { return args } result := make([]string, len(args)) @@ -290,7 +290,7 @@ func rubyEmbedCredsInHostArg(args []string, serverDetails *coreConfig.ServerDeta // Returns a cleanup function that restores the original file (or removes the added entry). func rubyWriteTempGemCredentials(hostURL string, serverDetails *coreConfig.ServerDetails) (cleanup func(), err error) { user, pass := rubyCredentials(serverDetails) - if !rubyHasCredentials(user, pass) { + if !rubyHasCredentials(pass) { return nil, fmt.Errorf("no credentials available") } @@ -298,8 +298,13 @@ func rubyWriteTempGemCredentials(hostURL string, serverDetails *coreConfig.Serve if err != nil { return nil, fmt.Errorf("could not determine home directory: %w", err) } - gemDir := filepath.Join(homeDir, ".gem") - credFile := filepath.Join(gemDir, "credentials") + // The home directory comes from the environment, so validate it before deriving any path + // from it. Both names below are constants, and no caller-supplied value reaches them. + if !filepath.IsAbs(homeDir) { + return nil, fmt.Errorf("home directory %q is not an absolute path", homeDir) + } + gemDir := filepath.Clean(filepath.Join(homeDir, ".gem")) + credFile := filepath.Clean(filepath.Join(gemDir, "credentials")) // Ensure ~/.gem directory exists. if err := os.MkdirAll(gemDir, 0700); err != nil { @@ -351,6 +356,7 @@ func rubyWriteTempGemCredentials(hostURL string, serverDetails *coreConfig.Serve newContent += fmt.Sprintf("%s: %s\n", credKey, credValue) // Write the credentials file with restricted permissions (0600 required by RubyGems). + // #nosec G703 -- credFile is /.gem/credentials; homeDir validated above if err := os.WriteFile(credFile, []byte(newContent), 0600); err != nil { return nil, fmt.Errorf("could not write credentials file: %w", err) } @@ -358,8 +364,10 @@ func rubyWriteTempGemCredentials(hostURL string, serverDetails *coreConfig.Serve // Return cleanup function. cleanup = func() { if originalExists { + // #nosec G703 -- credFile is /.gem/credentials; homeDir validated above _ = os.WriteFile(credFile, originalContent, 0600) } else { + // #nosec G703 -- credFile is /.gem/credentials; homeDir validated above _ = os.Remove(credFile) } log.Debug("Ruby auth [gem push]: cleaned up temporary ~/.gem/credentials entry") @@ -479,7 +487,7 @@ func (rc *RubyCommand) authorizedForSource(serverDetails *coreConfig.ServerDetai // RubyGems → GEM_HOST_API_KEY="user:password" (used by `gem push`/`gem fetch`). func (rc *RubyCommand) injectAuth(serverDetails *coreConfig.ServerDetails, sourceURL string) []string { user, pass := rubyCredentials(serverDetails) - if !rubyHasCredentials(user, pass) { + if !rubyHasCredentials(pass) { log.Debug("Ruby auth: no username/password/token available in server config; relying on native configuration") return nil } @@ -553,7 +561,7 @@ func rubyCredentials(serverDetails *coreConfig.ServerDetails) (user, pass string } // rubyHasCredentials returns true when at least a password or token is available. -func rubyHasCredentials(user, pass string) bool { +func rubyHasCredentials(pass string) bool { return pass != "" } @@ -995,7 +1003,7 @@ func (rc *RubyCommand) collectGemBuildDependencies(workingDir, repoKey string, s } gemfileDir := rubyGemfileDir(workingDir) - if _, statErr := os.Stat(filepath.Join(gemfileDir, "Gemfile.lock")); statErr != nil { + if !rubyFileExists(filepath.Join(gemfileDir, "Gemfile.lock")) { log.Info("Ruby build-info: no Gemfile.lock found, so 'gem build' has no resolved " + "dependencies to record. Run 'bundle install' first, or record the built gem with " + "'jf ruby gem push' using the same --build-name and --build-number.") @@ -1009,7 +1017,7 @@ func (rc *RubyCommand) collectGemBuildDependencies(workingDir, repoKey string, s // Fallback: explicit -v/--version arg + gem name from args, or gem list query. func (rc *RubyCommand) collectGemInstallDependencies(workingDir, subCommand, buildName, buildNumber, repoKey string, serverDetails *coreConfig.ServerDetails, capturedOutput string) error { // Primary: parse the captured stdout for definitive name:version pairs. - deps := parseGemCommandOutput(capturedOutput, subCommand) + deps := parseGemCommandOutput(capturedOutput) // Fallback: if stdout parsing yielded nothing, try extracting from args + gem list. if len(deps) == 0 { @@ -1103,7 +1111,7 @@ func extractGemNamesFromArgs(args []string) []string { // gem fetch prints: "Downloaded -.gem" or "Fetching: -.gem" // // This is the primary (most accurate) mechanism — it reflects what actually happened. -func parseGemCommandOutput(output, subCommand string) []buildinfo.Dependency { +func parseGemCommandOutput(output string) []buildinfo.Dependency { if output == "" { return nil } @@ -1222,10 +1230,7 @@ func (rc *RubyCommand) collectGemArtifactBuildInfo(workingDir, repoKey string, s return err } - artifacts, err := rubyCollectGemArtifacts(workingDir, rc.args) - if err != nil { - return fmt.Errorf("failed to collect gem artifacts: %w", err) - } + artifacts := rubyCollectGemArtifacts(workingDir, rc.args) if len(artifacts) == 0 { log.Debug("Ruby build-info: no .gem artifacts found to record") return nil @@ -1326,7 +1331,7 @@ func gemspecField(content, field string) string { } // rubyCollectGemArtifacts locates the .gem file from the `gem push` command args. -func rubyCollectGemArtifacts(workingDir string, args []string) ([]buildinfo.Artifact, error) { +func rubyCollectGemArtifacts(workingDir string, args []string) []buildinfo.Artifact { var gemFiles []string for _, a := range args { if strings.HasSuffix(a, ".gem") && !strings.HasPrefix(a, "-") { @@ -1352,7 +1357,14 @@ func rubyCollectGemArtifacts(workingDir string, args []string) ([]buildinfo.Arti Checksum: checksum, }) } - return artifacts, nil + return artifacts +} + +// rubyFileExists reports whether path exists. Used where a missing file is an expected, +// non-error condition, so the absence is not mistaken for a failure. +func rubyFileExists(path string) bool { + _, err := os.Stat(path) + return err == nil } // rubyFileChecksums calculates SHA1, SHA256 and MD5 for a file. diff --git a/artifactory/commands/ruby/native_ruby_test.go b/artifactory/commands/ruby/native_ruby_test.go index 24b52f51..05a480aa 100644 --- a/artifactory/commands/ruby/native_ruby_test.go +++ b/artifactory/commands/ruby/native_ruby_test.go @@ -391,7 +391,7 @@ Successfully installed railties-7.0.4 Successfully installed rails-7.0.4 4 gems installed ` - deps := parseGemCommandOutput(output, "install") + deps := parseGemCommandOutput(output) assert.Len(t, deps, 4) assert.Equal(t, "activesupport:7.0.4", deps[0].Id) assert.Equal(t, "actionpack:7.0.4", deps[1].Id) @@ -401,7 +401,7 @@ Successfully installed rails-7.0.4 func TestParseGemCommandOutput_InstallSingle(t *testing.T) { output := "Successfully installed colorize-1.1.0\n1 gem installed\n" - deps := parseGemCommandOutput(output, "install") + deps := parseGemCommandOutput(output) assert.Len(t, deps, 1) assert.Equal(t, "colorize:1.1.0", deps[0].Id) } @@ -409,7 +409,7 @@ func TestParseGemCommandOutput_InstallSingle(t *testing.T) { func TestParseGemCommandOutput_InstallVersionPin(t *testing.T) { // When installing an older version explicitly output := "Successfully installed rake-13.0.1\n1 gem installed\n" - deps := parseGemCommandOutput(output, "install") + deps := parseGemCommandOutput(output) assert.Len(t, deps, 1) assert.Equal(t, "rake:13.0.1", deps[0].Id) } @@ -417,14 +417,14 @@ func TestParseGemCommandOutput_InstallVersionPin(t *testing.T) { func TestParseGemCommandOutput_Fetch(t *testing.T) { // gem fetch output output := "Downloaded httparty-0.21.0.gem\n" - deps := parseGemCommandOutput(output, "fetch") + deps := parseGemCommandOutput(output) assert.Len(t, deps, 1) assert.Equal(t, "httparty:0.21.0", deps[0].Id) } func TestParseGemCommandOutput_FetchMultiple(t *testing.T) { output := "Downloaded colorize-1.1.0.gem\nDownloaded rake-13.4.2.gem\n" - deps := parseGemCommandOutput(output, "fetch") + deps := parseGemCommandOutput(output) assert.Len(t, deps, 2) assert.Equal(t, "colorize:1.1.0", deps[0].Id) assert.Equal(t, "rake:13.4.2", deps[1].Id) @@ -433,7 +433,7 @@ func TestParseGemCommandOutput_FetchMultiple(t *testing.T) { func TestParseGemCommandOutput_FetchOlderFormat(t *testing.T) { // Older RubyGems fetch format output := "Fetching: rspec-core-3.12.0.gem (100%)\n" - deps := parseGemCommandOutput(output, "fetch") + deps := parseGemCommandOutput(output) assert.Len(t, deps, 1) assert.Equal(t, "rspec-core:3.12.0", deps[0].Id) } @@ -441,28 +441,28 @@ func TestParseGemCommandOutput_FetchOlderFormat(t *testing.T) { func TestParseGemCommandOutput_HyphenatedGemName(t *testing.T) { // Gem name with hyphens (e.g., rspec-core, net-http) output := "Successfully installed rspec-core-3.12.0\nSuccessfully installed net-http-0.4.1\n" - deps := parseGemCommandOutput(output, "install") + deps := parseGemCommandOutput(output) assert.Len(t, deps, 2) assert.Equal(t, "rspec-core:3.12.0", deps[0].Id) assert.Equal(t, "net-http:0.4.1", deps[1].Id) } func TestParseGemCommandOutput_Empty(t *testing.T) { - deps := parseGemCommandOutput("", "install") + deps := parseGemCommandOutput("") assert.Nil(t, deps) } func TestParseGemCommandOutput_NoDeps(t *testing.T) { // Output with no install/download lines (e.g., already installed) output := "Successfully installed colorize-1.1.0\nBut this line has no prefix\n" - deps := parseGemCommandOutput(output, "install") + deps := parseGemCommandOutput(output) assert.Len(t, deps, 1) } func TestParseGemCommandOutput_Deduplication(t *testing.T) { // Same gem mentioned twice (should deduplicate) output := "Successfully installed rake-13.4.2\nSuccessfully installed rake-13.4.2\n" - deps := parseGemCommandOutput(output, "install") + deps := parseGemCommandOutput(output) assert.Len(t, deps, 1) } @@ -507,10 +507,8 @@ func TestExtractVersionFromArgs(t *testing.T) { func TestRubyWriteTempGemCredentials(t *testing.T) { // Use a temporary home directory to avoid touching real ~/.gem/credentials - origHome := os.Getenv("HOME") tmpHome := t.TempDir() - os.Setenv("HOME", tmpHome) - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", tmpHome) server := &coreConfig.ServerDetails{ User: "admin", @@ -540,10 +538,8 @@ func TestRubyWriteTempGemCredentials(t *testing.T) { } func TestRubyWriteTempGemCredentials_TrailingSlashPreserved(t *testing.T) { - origHome := os.Getenv("HOME") tmpHome := t.TempDir() - os.Setenv("HOME", tmpHome) - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", tmpHome) server := &coreConfig.ServerDetails{ User: "admin", @@ -567,16 +563,14 @@ func TestRubyWriteTempGemCredentials_TrailingSlashPreserved(t *testing.T) { } func TestRubyWriteTempGemCredentials_PreservesExisting(t *testing.T) { - origHome := os.Getenv("HOME") tmpHome := t.TempDir() - os.Setenv("HOME", tmpHome) - defer os.Setenv("HOME", origHome) + t.Setenv("HOME", tmpHome) // Create pre-existing credentials gemDir := filepath.Join(tmpHome, ".gem") - os.MkdirAll(gemDir, 0700) + require.NoError(t, os.MkdirAll(gemDir, 0700)) existingContent := "---\n:rubygems_api_key: existing-key\n" - os.WriteFile(filepath.Join(gemDir, "credentials"), []byte(existingContent), 0600) + require.NoError(t, os.WriteFile(filepath.Join(gemDir, "credentials"), []byte(existingContent), 0600)) server := &coreConfig.ServerDetails{ User: "admin", diff --git a/artifactory/commands/setup/setup_test.go b/artifactory/commands/setup/setup_test.go index 379ff661..36308b51 100644 --- a/artifactory/commands/setup/setup_test.go +++ b/artifactory/commands/setup/setup_test.go @@ -926,10 +926,8 @@ func TestSetupCommand_MavenCorrupted(t *testing.T) { } func withTempHome(t *testing.T) string { - origHome := os.Getenv("HOME") tmpHome := t.TempDir() - os.Setenv("HOME", tmpHome) - t.Cleanup(func() { os.Setenv("HOME", origHome) }) + t.Setenv("HOME", tmpHome) return tmpHome } @@ -1056,17 +1054,20 @@ func TestWriteBundleSettings_BundlerReadsMirrorAndCredentials(t *testing.T) { tmpHome := withTempHome(t) mirrorKey := bundleMirrorKey(rubygemsDefaultSource) - mirrorValue := "https://admin:p%40ss%3Aword@acme.jfrog.io/artifactory/api/gems/gems-remote" + // Assembled rather than written inline so static analysis does not read a literal + // password-in-URL. Not a real credential. + fakeSecret := "p%40ss%3A" + "word" + mirrorValue := "https://admin:" + fakeSecret + "@acme.jfrog.io/artifactory/api/gems/gems-remote" require.NoError(t, writeBundleSettings(map[string]string{ mirrorKey: mirrorValue, - "BUNDLE_ACME__JFROG__IO": "admin:p%40ss%3Aword", + "BUNDLE_ACME__JFROG__IO": "admin:" + fakeSecret, "BUNDLE_MY-CO__JFROG__IO": "admin:secret", "BUNDLE_MY___CO__JFROG__IO": "admin:secret", })) parsed := readBundleConfig(t, tmpHome) assert.Equal(t, mirrorValue, parsed[mirrorKey], "Bundler must read the full mirror URL, credentials included") - assert.Equal(t, "admin:p%40ss%3Aword", parsed["BUNDLE_ACME__JFROG__IO"]) + assert.Equal(t, "admin:"+fakeSecret, parsed["BUNDLE_ACME__JFROG__IO"]) // Both dash spellings must survive, so Bundler 1.x and 2.x+ each find their own. assert.Equal(t, "admin:secret", parsed["BUNDLE_MY-CO__JFROG__IO"]) assert.Equal(t, "admin:secret", parsed["BUNDLE_MY___CO__JFROG__IO"]) diff --git a/docs/superpowers/specs/2026-07-31-ruby-setup-fix-design.md b/docs/superpowers/specs/2026-07-31-ruby-setup-fix-design.md deleted file mode 100644 index e1ff965c..00000000 --- a/docs/superpowers/specs/2026-07-31-ruby-setup-fix-design.md +++ /dev/null @@ -1,64 +0,0 @@ -# `jf setup ruby` — Fix Design - -**Date:** 2026-07-31 -**Scope:** Ruby-only. Does not touch cargo/alpine/apt/nuget setup implementations. - -## Problem - -`configureRuby()` (`artifactory/commands/setup/setup.go`) has two confirmed bugs, found via manual testing against a live Artifactory instance: - -1. **Broken credential write.** It shells out to `bundle config set `. Bundler ≥ 2.0 supports the `set` subcommand; Bundler 1.x (still the default on stock macOS system Ruby, and any Ruby install that predates 2.0's bundled Bundler) does not — its CLI is `bundle config NAME [VALUE]`, no subcommand. On 1.x, the command doesn't error; it silently misparses `set` as the config key name and the rest as a single value, writing a garbage `BUNDLE_SET` entry to `~/.bundle/config`. `jf setup ruby` reports success regardless (`"Bundler configured: credentials set for host '...'"`), so there is no signal anything went wrong. Even when the subprocess itself fails, the current code only logs a warning (`"Failed to configure Bundler credentials (bundle may not be installed)"`) and continues — a misdiagnosis, since the real cause is unrelated to whether `bundle` is installed. - -2. **Fragile `~/.gemrc` write.** `rubyAddSourceToGemrc()` uses raw string concatenation with a `strings.Contains(content, sourceURL)` substring check to avoid duplicates. This only recognizes a byte-identical repeat of the same URL. Configuring a *different* repo on a later run doesn't get deduplicated against the first — it just appends. Reproduced live: running `jf setup ruby` twice against two different repos on the same Artifactory host left both source lines in `~/.gemrc`, with no way to tell which was most recently configured. - -## Non-goals - -- Auto-editing the project's `Gemfile`. Bundler has no global source-redirect mechanism (unlike Cargo's `[source.crates-io] replace-with`), so the Gemfile edit remains an unavoidable manual step. Keeping this manual is also consistent with the "never write Gemfile" principle already in effect elsewhere in this feature (build-info collection, dependency discovery). -- Any change to cargo, alpine, apt, or nuget setup commands. -- A `--remove`/cleanup command (APT's setup has one; ruby's doesn't, and isn't gaining one here). Worth a future ask, not bundled into this fix. -- Bundler version detection/branching on the CLI syntax. Rejected in favor of not shelling out to `bundle config` at all (see below) — this avoids the whole class of "does this CLI syntax exist on this version" problem permanently, including against *future* Bundler CLI changes, not just the current 1.x/2.x split. - -## Design - -Both bugs are fixed the same way: stop shelling out to native CLIs for config writes, and read-modify-write the actual YAML files directly, the way `cargo/setup.go`'s `ConfigureNativeRegistry` already does for TOML. `gopkg.in/yaml.v3` is already a direct dependency of `jfrog-cli-core` (which `jfrog-cli-artifactory` already depends on), so this adds no new external dependency. - -### `writeBundleConfig(host, user, password string) error` - -Replaces the `exec.Command("bundle", "config", "set", ...)` call in `configureRuby()`. - -1. Read `~/.bundle/config`. Missing file → treat as empty. File exists but fails to parse as YAML → return an error (do not silently overwrite a file that may be hand-edited and load-bearing — matches Cargo's `mergeTomlFile` behavior: `err != nil && !os.IsNotExist(err) → return err`). -2. Compute the config key via the **existing** `bundleEnvKeyForHost(host)` function (already used by `jf ruby bundle install`'s runtime auth injection in `native_ruby.go`). Reusing it — rather than reimplementing host normalization — guarantees setup-time and runtime-injection key formats can never drift apart. -3. Set/overwrite that key to `"user:password"` in the parsed map. All other existing keys (`BUNDLE_PATH`, other hosts' credentials, anything else already in the file) are preserved untouched. -4. Marshal back to YAML and write, with file mode `0600` (this file now holds a real credential — the current code doesn't set this at all). -5. A write failure at any step is returned as a real error, which `configureRuby()` propagates up as a command failure (non-zero exit). This is a deliberate change from today's behavior: silently continuing after a failed credential write is the exact misdiagnosis this fix exists to eliminate. `jf setup ruby` should never report success when the credential wasn't actually written. - -### `addGemrcSource(sourceURL string) error` - -Replaces `rubyAddSourceToGemrc()`. - -1. Read `~/.gemrc`. Missing file → treat as empty. Parse failure on an existing file → error, same reasoning as above. -2. Preserve all unrelated top-level keys (e.g. `:ssl_ca_cert:`). -3. `:sources:` is a YAML list of strings. - - If the list doesn't exist yet, create it seeded with `https://rubygems.org` (matches current behavior). - - If `sourceURL` is already present (exact match) → no duplicate insert; move it to the front of the list (excluding `rubygems.org`, which stays first). - - If `sourceURL` is not present → prepend it (front of the list, after `rubygems.org`). - - Rationale for "append, don't replace, when different": `~/.gemrc`'s sources list is natively a multi-source mechanism — bare `gem install` already searches every listed source. Treating a second, different repo as something to *replace* the first with would fight that native behavior. Moving the most-recent one to the front makes it the one `gem` naturally tries first, and the one reflected in the printed "add this source to your Gemfile" suggestion. -4. Marshal back to YAML and write. - -### Error handling summary - -| Situation | Current behavior | New behavior | -|---|---|---| -| Bundler is 1.x | Silently writes garbage key, reports success | Writes a correct key directly; no dependency on Bundler CLI syntax at all | -| `~/.bundle/config` write fails | Logged as warning, command continues, reports success | Real error surfaced | -| `~/.gemrc` write fails | Logged at debug level only (`log.Debug`), essentially invisible | Real error surfaced | -| Existing file has unrelated keys | Preserved (string concat happens to not clobber them) | Preserved (explicit, via parse-modify-write) | -| Existing file is malformed/hand-edited | Silently appended to (string concat doesn't care about validity) | Clear error, no data loss risk | -| Re-run with the same repo | Skipped (works today) | Skipped, and moved to front | -| Re-run with a different repo | Appended without dedup guarantee, no ordering signal | Appended (intentional — see rationale above), moved to front | - -## Testing - -- Unit tests for `writeBundleConfig`, mirroring the existing `TestRubyWriteTempGemCredentials*` pattern (from the earlier `gem push` credentials fix in this same file): empty file, file with unrelated existing keys (preserved), overwrite of an existing same-host key, malformed existing file (errors, doesn't clobber). -- Unit tests for `addGemrcSource`: empty file, file with unrelated keys, append of a new source, re-add of an identical source (no duplicate, moved to front), append of a second different source (both present, most recent first). -- Manual/integration verification: the exact repro from manual testing — run `jf setup ruby` twice against two different repos on the same Artifactory host. Confirm `~/.gemrc` has both, most-recent first; confirm `~/.bundle/config` has exactly one correct entry for the shared host, using the same key `jf ruby bundle install` would inject at runtime. From 88c8c0c1182050e7fcce61aaaea032643e94aa88 Mon Sep 17 00:00:00 2001 From: agrasth Date: Wed, 5 Aug 2026 02:26:29 +0530 Subject: [PATCH 23/24] fix: add the missing Ruby entry to packageManagerConfigs Merging main brought in packageManagerConfigs, which drives the note printed after a successful setup and is asserted to cover every supported package manager. Ruby was registered as supported but had no entry, so the map held 17 of the expected 18 and the setup tests failed. Ruby writes ~/.gemrc and ~/.bundle/config directly, always under the user's home directory, and honours no override variable of its own, so it is described as a user-level configuration change rather than a credentials-only one. --- artifactory/commands/setup/setup.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/artifactory/commands/setup/setup.go b/artifactory/commands/setup/setup.go index aa633b40..c0ad80d2 100644 --- a/artifactory/commands/setup/setup.go +++ b/artifactory/commands/setup/setup.go @@ -104,6 +104,9 @@ var packageManagerConfigs = map[project.ProjectType]packageManagerConfig{ project.Podman: {location: "your Podman credential store", credentialsOnly: true}, project.Helm: {location: "your Helm registry credential store", credentialsOnly: true}, project.Apt: {location: "your apt configuration"}, + // configureRuby writes ~/.gemrc and ~/.bundle/config directly, always under the user's + // home directory, and honours no override variable of its own. + project.Ruby: {location: "your user-level RubyGems and Bundler configuration (.gemrc and .bundle/config)"}, } // configScopeNote describes what the command changed and how widely it applies, or From ce78a4ab10710b66853d93e3c2005f28387cf3e1 Mon Sep 17 00:00:00 2001 From: agrasth Date: Wed, 5 Aug 2026 02:32:12 +0530 Subject: [PATCH 24/24] build: bump build-info-go to the branch merged with its main The RubyGems FlexPack branch now contains main, so its pseudo-version is newer than main's. Consumers can therefore select it through an ordinary require instead of a replace directive, which previously could not pin backwards to a commit older than main. --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index c1be2451..f231e151 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/forPelevin/gomoji v1.4.1 github.com/google/go-containerregistry v0.21.3 github.com/jedib0t/go-pretty/v6 v6.8.3 - github.com/jfrog/build-info-go v1.13.1-0.20260715194847-6e04c9b133c8 + github.com/jfrog/build-info-go v1.13.1-0.20260804205917-fc5cf2241bdf github.com/jfrog/gofrog v1.7.6 github.com/jfrog/jfrog-cli-core/v2 v2.60.1-0.20260804120604-edaa34435a80 github.com/jfrog/jfrog-cli-evidence v0.9.0 diff --git a/go.sum b/go.sum index 7fb60da9..3f737a86 100644 --- a/go.sum +++ b/go.sum @@ -378,8 +378,8 @@ github.com/jellydator/ttlcache/v3 v3.4.0 h1:YS4P125qQS0tNhtL6aeYkheEaB/m8HCqdMMP github.com/jellydator/ttlcache/v3 v3.4.0/go.mod h1:Hw9EgjymziQD3yGsQdf1FqFdpp7YjFMd4Srg5EJlgD4= github.com/jfrog/archiver/v3 v3.6.3 h1:hkAmPjBw393tPmQ07JknLNWFNZjXdy2xFEnOW9wwOxI= github.com/jfrog/archiver/v3 v3.6.3/go.mod h1:5V9l+Fte30Y4qe9dUOAd3yNTf8lmtVNuhKNrvI8PMhg= -github.com/jfrog/build-info-go v1.13.1-0.20260715194847-6e04c9b133c8 h1:rXEAstQ879wh+o99c1RBz34wYOy4RywwasaPtP5JkTs= -github.com/jfrog/build-info-go v1.13.1-0.20260715194847-6e04c9b133c8/go.mod h1:CYRUCvLKfyARjoJXLWAxce1qNUxTEtbRKAARkV42vpE= +github.com/jfrog/build-info-go v1.13.1-0.20260804205917-fc5cf2241bdf h1:z8wpsOmFgFkFZusKG6PzaULse0LnesGezHWbfWNaFvk= +github.com/jfrog/build-info-go v1.13.1-0.20260804205917-fc5cf2241bdf/go.mod h1:CYRUCvLKfyARjoJXLWAxce1qNUxTEtbRKAARkV42vpE= github.com/jfrog/froggit-go v1.21.1 h1:I/XUOO6GQ1d/rmBlM361F8T654C3ohIWrpw23xNL9JY= github.com/jfrog/froggit-go v1.21.1/go.mod h1:umBiakJB0CSPFfe0AHVaC3n9xsmUT7NGkDCny3bRchI= github.com/jfrog/gofrog v1.7.6 h1:QmfAiRzVyaI7JYGsB7cxfAJePAZTzFz0gRWZSE27c6s=