feat(deps): filter and group prompts - #18
Conversation
…semantic versioning Add support for tracking both latest stable and pre-release versions separately using semantic versioning. This enables users to see available pre-release updates alongside stable updates. Key changes: - New LatestVersions struct to hold both stable and prerelease versions - ResolveLatestVersions() method returns both version classes in a single lookup - ImageInfo struct extended with LatestPrerelease and PrereleaseUpdateAvailable fields - UI columns expanded to show separate stable and pre-release update indicators - New semverUpdateAvailable() function for proper semantic version comparison - Display paths now relative to working directory for better readability - Added comprehensive tests for pre-release version handling BREAKING CHANGE: ResolveLatest() behavior unchanged but ResolveLatestVersions() is the new recommended API for getting version information.
…m, and pnpm Implement a new `deps` command that generates dependency graphs for multiple package managers. Supports auto-detection of manifests, native package manager resolution with fallback to manifest parsing, filtering, and depth control. Includes comprehensive support for Go modules, Maven POMs, Gradle builds, npm/pnpm lockfiles with circular dependency detection and duplicate analysis. Features: - Auto-detect and resolve dependencies for go, maven, gradle, npm, pnpm - Native resolution via package manager tools with manifest fallback - Configurable depth limiting and dependency filtering - Duplicate and conflict detection - JSON export for programmatic consumption - Pretty-printed tree output with styled dependency metadata Refs: dependency graph analysis
Implement a new `deps update` subcommand that enables users to discover, resolve, and apply updates to direct dependencies across Go, npm, pnpm, container images, and Helm charts. Key features: - Interactive tree-based UI for selecting dependencies to update with filtering support - Version resolution and availability checking via package managers and image registries - Dry-run mode to preview changes without applying them - Check mode to list available updates without prompting - Support for MatchItem expressions to filter dependencies by name, manager, scope, and file path - Automatic git-aware manifest discovery that respects .gitignore - Enhanced pnpm support with dependency map parsing Changes include: - New UpdateOptions, UpdateCandidate, UpdateChoice, and UpdatePlan types - Image and Helm chart update discovery and application via imageupdate package - Interactive tree picker UI using bubbletea for dependency selection - Version sorting and filtering logic with semantic versioning support - Comprehensive test coverage for all update scenarios - Refactored manifest discovery to use git ls-files for better performance
…onal transitive graph support Replace native/manifest/auto resolution modes with a single offline-first approach that reads local manifests and lockfiles without running package-manager commands. Add --depth 0 support for transitive graphs via tool-specific resolvers (go mod graph, mvn dependency:tree, gradle dependencies) that fail fast with toolError when tools are unavailable, suggesting --depth 1 for offline output. Add new features: - deps diff subcommand to compare dependency graphs across git revisions - --flat flag to export flat node/edge lists instead of tree structure - --include-indirect flag for Go to include indirect requirements at --depth 1 - Support for image and Helm chart discovery from Kubernetes manifests - Comparison analysis with added/removed/updated change tracking Remove deprecated options: - --mode (native/manifest/auto) - --configuration (Gradle-specific) - --strict flag Breaking changes: - Default behavior now offline-only; use --depth 0 for transitive resolution - JSON export structure changes: roots/nodes/edges now conditional on --flat flag - Metadata.configurations replaced with Metadata.flat Refs: refactor to simplify resolution logic and improve offline-first user experience
…lt tags Tree rendering improvements computed before tree-type selection so flat and tree views stay consistent: - Drop the redundant manager prefix on child nodes; only the group root carries it (e.g. [go] appears once on the module root, not on every dependency). - Suppress default/std scope tags (direct, require, compile, dependencies) and surface only meaningful ones (indirect, replaced, dev, optional, local). - Group image/helm dependencies by Namespace -> Kind instead of a flat list. - Print package roots (go.mod, etc.) as repo-relative paths instead of absolute. Namespace/kind/resource/container are carried as out-of-band node properties so the JSON model and statistics are unchanged.
Drop the synthetic "container images"/"helm charts" root wrappers from the rendered tree and group all kubernetes-derived dependencies (images + helm) by: Namespace (☸ icon) > Kind (white) > resource name (shortened path) > image/chart Image and helm dependencies in the same namespace now share one namespace node. The shortened path keeps the last two segments to disambiguate sibling files (e.g. manifests/ vs golden/). JSON model and statistics are unchanged.
Add automatic staging of updated dependency files after successful updates. When a dependency update is written, the affected manifest and lockfile(s) are now staged with `git add`. Dry-run and check modes do not stage files. New UpdatePlan fields track staged files and any staging errors. The stageUpdatedFiles function handles manager-specific file patterns (go.mod/go.sum, package.json/package-lock.json, etc.) and only stages files that exist. Pretty() and Row() methods updated to display staging status. Refs the update workflow documentation change describing this behavior.
Render each shared dependency once at its resolved (shallowest BFS) occurrence instead of repeating it under every parent. The resolved node is tagged with the count of other parents and each parent that hid a duplicate gets a trailing "(and N other dependencies)" marker. The model and JSON carry the new other_parents/hidden_duplicates counts. --show-duplicates restores the full per-parent repetition with dup:N tags. Stats, --flat node/edge lists, and deps diff stay on the full uncollapsed graph (collapse is forced off internally for diffs).
Discover local Helm chart directories (Chart.yaml) in addition to Flux
HelmRelease manifests. Each chart becomes a dependency root carrying its
declared subchart dependencies (helm, version recorded verbatim from the
Chart.yaml range) plus the container images referenced in values.yaml and
templates/ (image). Templated values (`{{ ... }}`) are skipped since they
cannot be resolved offline.
Chart discovery is a filesystem walk independent of the git-backed
k8s-manifest scanner, so a chart directory outside a git repo still
resolves; image-discovery errors now degrade to a warning when other
roots resolved. Vendored subcharts under a parent chart's charts/ dir are
skipped. Chart roots render package-style (manager-prefixed children),
distinct from the namespace/kind grouping used for k8s manifest images.
At --depth other than 1 (mirroring go/maven/gradle), dependency scanning now recurses into remote dependencies, backed by a persistent cache under the user cache dir: - Helm charts: each subchart is fetched from its Helm repository (index resolved via SemVer range, chart .tgz loaded with the Helm SDK chart loader), and its own subcharts and values/template images are recursed. - Container images: the base image is resolved from the OCI base-image label and from the Dockerfile FROM directives in the image's source repository (located via the OCI source label or a ghcr/quay heuristic, cloned into the cache), then recursed. The cache (remote_cache.go) keeps immutable artifacts (git clones, chart archives, image config by digest) long and revalidates indexes/tags daily, with negative caching and singleflight. Remote failures degrade to per-node warnings; only cache initialization is fatal. --depth 1 stays fully offline. Helm is pinned to v3.17.x and only pkg/chart/loader is used (index.yaml parsed directly): v3.18+ bumps distribution/v3 past what registry-scanner needs, and pkg/repo transitively requires a k8s API removed in the pinned k8s release.
…s under deps command Consolidate image/helm and package manager (go/npm/pnpm) update logic into a single `deps update` command with unified filtering, version resolution, and interactive selection. The legacy `images update` command now delegates to `deps update --manager image,helm`. Key changes: - Add --latest and --version flags to resolve dependencies non-interactively - Add resource filters (--kind, --namespace, --name, --selector) and name patterns (--image, --chart) for image/helm targets - Make expression argument optional; empty expression matches all dependencies - Support Flux v2 HelmRelease spec.chartRef (OCIRepository/HelmChart) in addition to inline spec.chart - Extract version math, matching, plan rendering, resolution, and prompts into separate files - Add helm credentials support (basic auth, TLS) for private chart repositories - Refactor imageupdate discovery into DiscoverRepoTargets and Filter functions - Add comprehensive tests for --latest, --version, resource filters, and chartRef resolution BREAKING CHANGE: `images update` is deprecated; use `deps update --manager image,helm` instead. The `deps update` expression argument is now optional (was required).
Apply effective Kustomize namespaces to discovered targets, distinguish exact-file scans from directory scans, and report untracked YAML files. Preserve chart source-resolution errors so callers can surface actionable diagnostics.
Expose Kubernetes metadata filters for image and Helm dependencies, while preserving positional paths and root help behavior. Restore implicit scan routing for bare paths, flags, and empty invocations.
Add Kubernetes resource filtering for image and Helm scans, with actionable errors for untracked manifest targets. Reuse published-version lookups across duplicate occurrences while preserving per-occurrence updates. Collapse package-manager duplicates silently by default and retain duplicate analysis only with --show-duplicates. BREAKING CHANGE: Remove Node.other_parents and Node.hidden_duplicates and their rendered markers; duplicate statistics and sections are no longer produced by default.
Expose MatchItem dependency filters through `deps update` and combine them with positional patterns. Deduplicate identical version prompts so one confirmation updates repeated declarations consistently. BREAKING CHANGE: rename `UpdateOptions.Expression` to `Filters` and change `VersionSelector` to receive `UpdateVersionPrompt`.
WalkthroughThis change adds dependency scanning, comparison, remote resolution, update workflows, cache warming, image and Helm discovery, and related CLI commands. It also updates documentation, path display handling, output formats, and dependency declarations. ChangesCLI entrypoints and repository integration
Dependency scanning and comparison
Remote resolution and update workflows
Cache warming implementation
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (20)
go.mod-22-22 (1)
22-22: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winOther (CWE-1395)
Reachability: External
Reachability path
● Entry deps/remote_cache.go:1 golang.org/x/sync │ ▼ ● Sink go.modUpgrade Helm before merge.
helm.sh/helm/v3 v3.17.3is affected by chart-driven security advisories used bydeps/remote_helm_client.gowhen resolving and loading remote Helm charts. Upgrade to a patched version outside all affected ranges, such as Helm v3.17.4+ or v3.18.4+. Avoid v3.18.0 through v3.18.3 for the code-injection advisory. Verify the module graph after the upgrade.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@go.mod` at line 22, Upgrade the helm.sh/helm/v3 dependency from v3.17.3 to a patched release such as v3.17.4+ or v3.18.4+, avoiding v3.18.0–v3.18.3, then verify the resulting Go module graph and ensure deps/remote_helm_client.go remains compatible.Source: Linters/SAST tools
deps/compare.go-71-95 (1)
71-95: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPreserve scan warnings in comparison output.
Comparedrops warnings from both exports.Comparison.Prettyalso skips warning rendering when no dependency changed. A partial or degraded scan can therefore appear as a clean comparison.
deps/compare.go#L71-L95: Copy warnings from each non-nil input export intocomparison.Warnings.deps/compare_pretty.go#L34-L45: Append"No dependency changes"without returning, then rendercomparison.Warnings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/compare.go` around lines 71 - 95, Update Compare in deps/compare.go (lines 71-95) to copy warnings from each non-nil input export into comparison.Warnings while preserving the existing dependency comparison. Update Comparison.Pretty in deps/compare_pretty.go (lines 34-45) to append “No dependency changes” without returning, then render comparison.Warnings even when no dependency changes exist.deps/update_image.go-112-121 (1)
112-121: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
imageVersionOnlyreturns the digest hex instead of the tag.The function locates the tag separator before it strips the digest.
strings.LastIndex(value, ":")finds the colon inside@sha256:..., so:
nginx:1.25.3@sha256:abcdreturnsabcdinstead of1.25.3.nginx@sha256:abcdreturnsabcdinstead of an empty version.The
@strip on line 115 runs after the slice, so it never removes the digest.stripImageVersionon lines 102-110 already handles this correctly by stripping@first.This value becomes
UpdateCandidate.Current, soUpdatePlan.OldVersionis wrong andselectedVersionIsCurrentcompares against a digest fragment for digest-pinned images.🐛 Proposed fix
func imageVersionOnly(value string) string { + if at := strings.Index(value, "@"); at >= 0 { + value = value[:at] + } if i := imageTagSeparator(value); i >= 0 { - version := value[i+1:] - if at := strings.Index(version, "@"); at >= 0 { - version = version[:at] - } - return version + return value[i+1:] } - return value + return "" }Confirm the intended return for an image with no tag. The current code returns the whole reference, which reads as a version in plan output.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/update_image.go` around lines 112 - 121, Update imageVersionOnly to remove any `@digest` suffix before locating the tag separator, reusing the ordering established by stripImageVersion. Return only the tag for tagged references, and return an empty version for digest-only or untagged references instead of returning the full image reference.cmd/repomap/images.go-14-25 (1)
14-25: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winHandle digest suffixes and registry ports in
versionOnly.
strings.LastIndexfinds the colon inside the digest, not the tag separator. Forghcr.io/acme/app:1.2.3@sha256:abc…the function returns the digest hex, because the@strip runs on a substring that no longer contains@. A registry port without a tag, for exampleregistry:5000/app, returns5000/app.The result feeds
semverUpdateAvailableincmd/repomap/images_list.go.semver.NewVersionthen fails for the digest hex, the code falls back tolatest != current, and the row reports a false update for every digest-pinned image.Strip the digest first, then take the colon after the last
/.🐛 Proposed fix
func versionOnly(currentValue string) string { - if i := strings.LastIndex(currentValue, ":"); i >= 0 { - v := currentValue[i+1:] - if at := strings.Index(v, "@"); at >= 0 { - v = v[:at] - } - return v - } - return currentValue + v := currentValue + if at := strings.Index(v, "@"); at >= 0 { + v = v[:at] + } + // Only a colon after the last path separator is a tag separator; an + // earlier one belongs to a registry host:port. + start := strings.LastIndex(v, "/") + 1 + if i := strings.LastIndex(v[start:], ":"); i >= 0 { + return v[start+i+1:] + } + return v }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/repomap/images.go` around lines 14 - 25, Update versionOnly to remove any digest suffix at “@” before parsing the tag, then locate the final colon only in the portion after the last “/” so registry ports without tags are preserved correctly. Return the resulting tag/version when a post-path colon exists, otherwise return the digest-stripped value unchanged.imageupdate/chartref.go-128-150 (1)
128-150: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not mutate the target before the version check fails.
Lines 130-145 write
RepoURL,IsOCI,File,FieldLine,FieldJSONPath,CurrentValue, andChartNameontot. Line 146 then returns an error whenCurrentValueis empty. The target keeps every partial mutation.
Resolveis documented as idempotent and returns early whent.RepoURL != ""(imageupdate/sourceref.goline 109). Because this error path already setRepoURL, a secondResolvecall on the same target returnsnil.DiscoverTargetsrecordsSourceErrand keeps the target (imageupdate/discover.golines 72-79);checkInfoincmd/repomap/images_list.goline 169 then callsResolveagain on it. That second call reports success, the "no editable version" error disappears, and version resolution runs with an emptyCurrentValue.Validate the version before you write to
t.🐛 Proposed fix
+ if src.version == "" { + return fmt.Errorf("HelmRelease %s/%s references %s %q which has no editable version (uses semver/digest, not a tag)", + t.Ref.Namespace, t.Ref.Name, src.kind, t.ChartRefName) + } + switch src.kind { case "OCIRepository": t.RepoURL = src.repoURL t.IsOCI = src.isOCI case "HelmChart": repo, err := idx.lookupHelmRepository(src.srcRefName, chartSourceNamespace(src, wantNS), t.Ref) if err != nil { return fmt.Errorf("HelmChart %s/%s: %w", src.srcRefNamespace, src.chartName, err) } t.RepoURL = repo.URL t.IsOCI = repo.IsOCI } t.File = src.file t.FieldLine = src.versionLine t.FieldJSONPath = src.versionPath t.CurrentValue = src.version t.ChartName = src.chartName - if t.CurrentValue == "" { - return fmt.Errorf("HelmRelease %s/%s references %s %q which has no editable version (uses semver/digest, not a tag)", - t.Ref.Namespace, t.Ref.Name, src.kind, t.ChartRefName) - } return nil🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@imageupdate/chartref.go` around lines 128 - 150, Update the target-population flow in Resolve so src.version is validated for an editable, non-empty version before assigning any fields to t. Keep the existing error behavior for unsupported semver/digest references, and only mutate RepoURL, IsOCI, File, FieldLine, FieldJSONPath, CurrentValue, and ChartName after that validation succeeds.cmd/repomap/images_list.go-166-192 (1)
166-192: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winBuild
infoaftersourceIndex.Resolve, not before.Line 167 snapshots
tintoinfobefore line 169 resolves it. For a chartRef HelmRelease,ResolvesetsCurrentValue,File,FieldLine, andChartNameon the target (imageupdate/chartref.golines 141-145); before resolution those fields are empty.imageupdate/chartref_test.goline 47 asserts exactly that.Two results follow for every chartRef chart row:
info.Currentstays empty, so the Current column is blank while the Latest column shows a version and a green update mark.info.Filekeeps the HelmRelease file instead of the OCIRepository/HelmChart file that holds the version literal, so the listed file does not match the filedeps updateedits.The update flags at lines 189-190 already read the resolved
t, which makes the row internally inconsistent.🐛 Proposed fix
func checkInfo(ctx context.Context, resolver *imageupdate.Resolver, sourceIndex *imageupdate.SourceIndex, t imageupdate.UpdateTarget, displayFile string, tk *task.Task) ImageInfo { - info := baseInfo(t, true, displayFile) if t.Kind == imageupdate.TargetChart { if err := sourceIndex.Resolve(&t); err != nil { tk.Warnf("source unresolved: %v", err) + info := baseInfo(t, true, displayFile) info.Error = err.Error() return info } } + // Resolve redirects a chartRef target onto the source object that holds the + // version literal, so snapshot the row only after it succeeds. + info := baseInfo(t, true, displayFile) tk.Infof("looking up latest stable and pre-release versions")The caller computes
displayFilefrom the pre-resolutiontarget.File. Recompute it from the resolvedt.Fileso the listed file matches the edit anchor. That needs thedisplayPathfunction insidecheckInfo:// cmd/repomap/images_list.go — pass displayPath instead of a precomputed string group.Add(taskName(target, displayPath(target.File)), func(ctx flanksourceContext.Context, tk *task.Task) (int, error) { infos[idx] = checkInfo(ctx, resolver, sourceIndex, target, displayPath, tk) return idx, nil })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/repomap/images_list.go` around lines 166 - 192, Move the baseInfo call in checkInfo until after sourceIndex.Resolve(&t) completes, so chart targets use resolved CurrentValue, File, FieldLine, and ChartName. Change checkInfo to receive the displayPath function rather than a precomputed displayFile, then compute the display file from the resolved t.File before building info; update its caller accordingly while preserving non-chart behavior.tracked_yaml.go-16-25 (1)
16-25: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse NUL-delimited git output for path discovery.
TrackedYAMLContents()splitsgit ls-filesoutput on\n, butgit ls-filesquotes paths with non-ASCII bytes, newlines, or double quotes by default. A quoted path is passed as an escaped filename toReadFileWithFallbackand skipped, so discovery omits YAML files that should be included.Use
git ls-files -zand split on\x00; the scan and chart catalog paths already use this pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tracked_yaml.go` around lines 16 - 25, Update TrackedYAMLContents to invoke git ls-files with the -z option and split result.Stdout on NUL characters instead of newlines. Preserve the existing trimming, YAML filtering, and file processing behavior so paths containing special characters are passed unescaped to ReadFileWithFallback.deps/discover.go-132-151 (1)
132-151: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply
ignoredDirsin the git discovery path.
walkManifestFilesskipsvendor,node_modules,build,dist,target, and.gradle.gitManifestFilesskips none of them.git ls-files --others --exclude-standardonly removes gitignored paths, so a committedvendor/ornode_modules/tree yields manifests here that the filesystem walk would drop. The discovered project set then changes based on whether git is available.Filter the git results with the same rule.
🐛 Proposed fix to share the ignore rule
var files []string for _, rel := range strings.Split(string(out), "\x00") { if rel == "" { continue } - name := filepath.Base(filepath.FromSlash(rel)) + local := filepath.FromSlash(rel) + if hasIgnoredDir(local) { + continue + } + name := filepath.Base(local) if managerForManifest(name) == "" { continue } - files = append(files, filepath.Join(root, filepath.FromSlash(rel))) + files = append(files, filepath.Join(root, local)) }Add the helper:
func hasIgnoredDir(rel string) bool { for _, part := range strings.Split(filepath.ToSlash(filepath.Dir(rel)), "/") { if ignoredDirs[part] { return true } } return false }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/discover.go` around lines 132 - 151, Apply the existing ignoredDirs rule to gitManifestFiles so committed vendor, node_modules, build, dist, target, and .gradle paths are excluded consistently with walkManifestFiles. Add and use a shared hasIgnoredDir helper when filtering each relative git result, before managerForManifest and appending the path.deps/npm.go-132-176 (1)
132-176: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftUnbounded tree expansion for hoisted npm lockfiles.
packageLockChildrenre-expands the full subtree of every dependency at every parent that declares it.seenonly stops cycles along a single path; it does not stop repeats across sibling branches, and noMaxDepthis applied during construction. A hoistedpackage-lock.jsonin which many packages depend on the same popular package therefore materializes that package's subtree once per referencing parent, so node count grows combinatorially beforefilterAndPrunecan bound anything.
buildTreeFromEdgeGraphindeps/edgegraph.goalready solves this by expanding a key only at its shallowest BFS occurrence and by pruning pastMaxDepthduring construction. Consider building anedgeGraphfromlock.Packagesand reusing that path, or at minimum threadOptions.MaxDepthinto this recursion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/npm.go` around lines 132 - 176, Bound npm lockfile tree construction in packageLockChildren by applying Options.MaxDepth during recursion and preventing repeated subtree expansion across sibling branches, preferably by building an edgeGraph from lock.Packages and reusing buildTreeFromEdgeGraph’s shallowest-BFS expansion behavior. Ensure hoisted dependencies are not materialized once per referencing parent before filterAndPrune runs.deps/filter.go-75-97 (1)
75-97: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Statistics.Circularunder-counts circular references.The
if node.Circularcheck sits inside theif _, exists := nodeMap[node.ID]; !existsblock. A circular occurrence always repeats an ancestor ID, and that ancestor was inserted intonodeMapearlier as a non-circular node. The circular occurrence therefore hits theexistsbranch and is never counted, socircular_referencesreports 0 for the cycle case thatTestBuildTreeTerminatesOnCycleconstructs. Move the circular tally outside the dedup guard.🐛 Proposed fix
if _, exists := nodeMap[node.ID]; !exists { nodeMap[node.ID] = FlatNode{ ... } stats.ByManager[node.Manager]++ if node.Depth > stats.MaxDepth { stats.MaxDepth = node.Depth } - if node.Circular { - circular++ - } } + if node.Circular { + circular++ + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/filter.go` around lines 75 - 97, Move the node.Circular tally in the node-processing logic outside the nodeMap deduplication guard, while keeping node insertion and manager/depth statistics inside it. Ensure every circular occurrence increments the circular count, including repeated ancestor IDs handled by the exists branch, so Statistics.Circular reports cycle references correctly.deps/go_graph.go-105-134 (1)
105-134: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
applyGoModMetadatamarks deeper occurrences as direct.
requiresis keyed by module path only, andwalkapplies it at every depth.go mod graphlists MVS inputs, so the same module path appears at several versions and at several depths. Every occurrence of a required path receivesDirect = trueandScope = "require", including nodes at depth 2 or deeper. This disagrees withresolveGoManifestindeps/go.go, which marks only depth-1 children as direct, and it corrupts direct-dependency counts and filters.Key the lookup by path and version, and set
Directonly for depth-1 nodes.🐛 Proposed fix
var walk func(n *Node) walk = func(n *Node) { if n == nil { return } if req, ok := requires[n.Name]; ok { - n.Direct = !req.Indirect + n.Direct = !req.Indirect && n.Depth == 1 if req.Indirect { n.Scope = "indirect" } else { n.Scope = "require" } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/go_graph.go` around lines 105 - 134, Update applyGoModMetadata to track traversal depth and mark Direct and Scope as direct/require only for depth-1 children of root; deeper matching nodes must not receive direct metadata. Key requires by both module path and version, and use that path-version lookup when applying replacement and dependency metadata while preserving indirect handling for direct children.deps/pnpm.go-110-142 (1)
110-142: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftThe recursion enumerates every path and ignores
MaxDepth.
buildPNPMNodecallscloneBoolMap(seen)for each child at Line 138. Theseenset is therefore per-path, not global. A package reached through N distinct paths is expanded N times, and each expansion clones the whole set again.npm and pnpm graphs share subtrees heavily. A diamond of depth d produces on the order of 2^d expansions. A real
pnpm-lock.yamlwith a few thousand packages can consume very large amounts of CPU and memory in a single scan.There is no depth cap on this path.
resolvePNPMManifesttakes noOptions, soOptions.MaxDepthis never consulted. The Go, Maven, and Gradle resolvers all route throughbuildTreeFromEdgeGraph(edgeTreeOptions{Graph: graph, MaxDepth: opts.MaxDepth})and get truncation for free.Build an
edgeGraphfrom the lockfile and reusebuildTreeFromEdgeGraph. That fixes the depth cap and the repeated expansion together, and it makes pnpm consistent with the other managers.Run the following script to confirm the resolver signatures and how
MaxDepthreaches the other managers:#!/bin/bash # Description: Compare pnpm resolution against the edge-graph resolvers and locate the dispatch site. set -euo pipefail rg -nP --type=go -C3 '\bresolvePNPMManifest\s*\(|\bresolveNPMManifest\s*\(' rg -nP --type=go -C6 'buildTreeFromEdgeGraph\(' ast-grep run --pattern 'func cloneBoolMap($$$) { $$$ }' --lang go deps🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/pnpm.go` around lines 110 - 142, Replace the recursive buildPNPMNode traversal with an edgeGraph constructed from the pnpm lockfile, then resolve it through buildTreeFromEdgeGraph using edgeTreeOptions with opts.MaxDepth. Update resolvePNPMManifest and its dispatch callers to accept and propagate Options, and remove the per-path cloneBoolMap recursion so shared subtrees are expanded once while respecting the depth cap.deps/helm_credentials.go-93-112 (1)
93-112: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSet a timeout and surface failures to load TLS material.
Two problems in
httpClientForCreds:
- The returned client has no
Timeout. A chart repository that accepts the connection and then stalls blocks the scan for as long as the operating system permits.http.DefaultClienton Lines 95 and 81 has the same property. Set an explicitTimeout, or rely on a context deadline applied by the caller.- Lines 99-110 discard every error. If
CAFileis unreadable, or contains no valid PEM, orLoadX509KeyPairfails, the client silently falls back to system roots and no client certificate. The user configured a pinned CA and mutual TLS, and receives neither, with no diagnostic. Return the error, or record a warning.🛠️ Proposed change for the timeout
- return &http.Client{Transport: &http.Transport{TLSClientConfig: tlsCfg}} + return &http.Client{ + Timeout: helmHTTPTimeout, + Transport: &http.Transport{TLSClientConfig: tlsCfg}, + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/helm_credentials.go` around lines 93 - 112, Update httpClientForCreds to enforce an explicit request timeout for both custom and default clients, or ensure its callers apply an equivalent context deadline. Stop silently ignoring CA and client-certificate loading failures: propagate the error through the function’s return contract or emit a warning while preserving the configured TLS material behavior, including unreadable/invalid CA files and failed LoadX509KeyPair calls.deps/maven.go-23-23 (1)
23-23: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLoad arbitrary POM property elements under
dependencies
xml:"properties>*"makespom.Propertiesalways empty because Go’s XML decoder matches the final path segment literally, so an element named*never occurs. Copy this pattern around line 64 into all dependencyVersionfields indeps/maven_tree.goso dependency versions resolved from POM properties are not left as unexpanded${...}values.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/maven.go` at line 23, Update the dependency Version XML mappings in maven_tree.go to use the working arbitrary-property-element pattern already present near line 64, rather than xml:"properties>*". Apply it to every dependency Version field so POM property references are loaded and can be expanded instead of remaining as ${...} values.deps/gradle_tree.go-49-63 (1)
49-63: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the Gradle wrapper search to the scan root.
gradleCommandwalks fromproject.Dirto the filesystem root. If nogradlewexists under the selected dependency scan root, it can execute a wrapper from an unrelated parent directory, such as$HOME/gradlewor/gradlew. Stop the walk once the search leaves the scan root.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/gradle_tree.go` around lines 49 - 63, Update gradleCommand so its upward search is bounded by the dependency scan root, stopping before checking directories outside that root; retain the existing wrapper detection within the root and return "gradle" when no in-root gradlew is found.deps/pnpm.go-65-82 (1)
65-82: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winThe bare-name index entry is nondeterministic.
Lines 76-78 store a package under its bare name when that key is free. The enclosing loop ranges over a Go map, and Go randomizes map iteration order. When a lockfile contains several versions of the same package, which version wins is different on each run.
buildPNPMNodeuses that entry as a fallback at Line 120 whenever the exactname@versionlookup misses. The resulting node then gets a differentSourceand a different child set between runs of the same scan.This breaks reproducible output.
deps diffcompares two exports and will report changes that do not exist.Choose the fallback deterministically. Sort the keys before indexing, or select the highest version.
🐛 Proposed fix
func pnpmPackageIndex(raw map[string]any) map[string]pnpmPackage { out := map[string]pnpmPackage{} for _, section := range []string{"packages", "snapshots"} { - for key, value := range asMap(raw[section]) { + entries := asMap(raw[section]) + keys := make([]string, 0, len(entries)) + for key := range entries { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + value := entries[key] name, version := splitPNPMPackageKey(key) if name == "" { continue } data := asMap(value) pkg := pnpmPackage{Name: name, Version: version, Key: key, Data: data, Children: pnpmDeps(data)} out[pnpmPackageID(name, version)] = pkg if _, ok := out[name]; !ok { out[name] = pkg } } } return out }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/pnpm.go` around lines 65 - 82, Make the bare-name fallback in pnpmPackageIndex deterministic when multiple versions exist. Sort package keys before iterating the packages and snapshots maps, or consistently select the highest version, so the entry used by buildPNPMNode after an exact lookup miss always yields the same package and children.deps/helm_credentials.go-79-91 (1)
79-91: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSensitive Data Exposure (CWE-319): Cleartext Transmission of Sensitive Information
Reachability: Internal
Restrict Basic credentials to HTTPS endpoints without downgrade paths.
authorizesends Basic auth when the repository URL matches, includinghttp://repos, and the returned client uses Go’s default redirect policy. Anhttps://repository can retain theAuthorizationheader across a same-host redirect tohttp://; the custom client never checksCheckRedirectat all. Reject matching credentials unless the initial request ishttps, and refuse downgrade redirects by clearingAuthorizationand returning an error when the redirected URL is nothttps.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/helm_credentials.go` around lines 79 - 91, The authorize method must only apply matching credentials when req.URL uses HTTPS; otherwise return http.DefaultClient without setting Basic auth. Update the client returned by httpClientForCreds to enforce a CheckRedirect policy that clears Authorization and returns an error whenever a redirect target is not HTTPS, while preserving safe HTTPS redirects.Source: Linters/SAST tools
deps/scan.go-70-77 (1)
70-77: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDegrade chart-discovery failures to warnings, as image discovery does.
chartErraborts the whole scan.imageErronly aborts when nothing else resolved (Lines 88-96).discoverChartFilespropagatesfilepath.WalkDirerrors verbatim, so one unreadable directory fails a scan that already resolved Go, Maven, or npm roots. Apply the same degrade policy tochartErr.🛠️ Proposed fix
+ var chartErr error if scanImages { ... if matcher.IsEmpty() { - chartRoots, chartWarnings, chartErr := discoverChartDependencyRoots(absPath, opts.Managers) + var chartRoots []*Node + var chartWarnings []Warning + chartRoots, chartWarnings, chartErr = discoverChartDependencyRoots(absPath, opts.Managers) warnings = append(warnings, chartWarnings...) - if chartErr != nil { - return nil, chartErr - } roots = append(roots, chartRoots...) }Then handle
chartErrnext toimageErr: return it only whenlen(roots) == 0, otherwise append it as aWarning.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/scan.go` around lines 70 - 77, Update the chartErr handling in the scan flow around discoverChartDependencyRoots to match imageErr: if len(roots) == 0, return chartErr; otherwise append chartErr as a Warning and continue scanning. Preserve the existing chartRoots and chartWarnings accumulation for successful or partially successful discovery.deps/runner.go-27-39 (1)
27-39: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound external command execution with a default timeout.
ExecRunner.Runinherits whatever context the caller supplies.ScanandUpdateforward the caller context, which commonly has no deadline.mvn dependency:treeandgradle dependenciesreach the network and can hang. A hang insideresolveProjectsWithTasksstalls the whole scan with no bound and no output.Add a configurable default deadline on the runner.
🛠️ Proposed fix
-type ExecRunner struct{} +type ExecRunner struct { + // Timeout bounds a single command. Zero selects defaultCommandTimeout. + Timeout time.Duration +} + +const defaultCommandTimeout = 5 * time.Minute -func (ExecRunner) Run(ctx context.Context, spec Command) (CommandResult, error) { +func (r ExecRunner) Run(ctx context.Context, spec Command) (CommandResult, error) { + timeout := r.Timeout + if timeout == 0 { + timeout = defaultCommandTimeout + } + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() cmd := exec.CommandContext(ctx, spec.Name, spec.Args...)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/runner.go` around lines 27 - 39, Update ExecRunner.Run to apply a configurable default timeout when the supplied context has no deadline, while preserving caller-provided deadlines. Create the derived context before exec.CommandContext and ensure its cancellation is released after command execution. Keep the existing command output and error handling unchanged.deps/remote_helm_client.go-175-186 (1)
175-186: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound the decompressed size of chart archives.
io.Copywrites the tar entry to disk with no limit. The//nolint:goseccomment states that chart archives are size-bounded, but that bound applies to the compressed bytes thatcache.Fetchreturned. Gzip expansion is not bounded by it. A 10 MB archive from a remote index can expand to many gigabytes and exhaust the disk. The archive host is chosen by the scannedChart.yaml.Track a running total across entries and stop when it exceeds a cap.
🛠️ Proposed fix
+// maxChartExtractBytes caps the total expanded size of a chart archive. +const maxChartExtractBytes = 512 << 20 + func extractTarGz(data []byte, dest string) error { gz, err := gzip.NewReader(bytes.NewReader(data)) if err != nil { return err } defer func() { _ = gz.Close() }() tr := tar.NewReader(gz) + var written int64 for {f, err := os.Create(target) if err != nil { return err } - if _, err := io.Copy(f, tr); err != nil { //nolint:gosec // chart archives are size-bounded + n, err := io.Copy(f, io.LimitReader(tr, maxChartExtractBytes-written)) + written += n + if err != nil { _ = f.Close() return err } + if written >= maxChartExtractBytes { + _ = f.Close() + return fmt.Errorf("chart archive exceeds %d bytes when extracted", maxChartExtractBytes) + } if err := f.Close(); err != nil { return err }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/remote_helm_client.go` around lines 175 - 186, In the chart extraction flow around the tar-entry copy and file handling, track cumulative decompressed bytes across all entries and enforce a maximum archive-size cap. Replace unrestricted io.Copy with a bounded copy that cannot write beyond the remaining allowance, stop and return an error when the cap is exceeded, and preserve cleanup of the open file on copy failure. Keep the existing close-error handling and compressed cache.Fetch limit unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 85a6e85b-3399-4985-8cd7-0667623561a2
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (95)
.gitignore.grite/export.jsonREADME.mdcmd/repomap/deps.gocmd/repomap/deps_diff.gocmd/repomap/deps_diff_test.gocmd/repomap/deps_test.gocmd/repomap/images.gocmd/repomap/images_list.gocmd/repomap/images_list_test.gocmd/repomap/images_test_helpers.gocmd/repomap/images_update.gocmd/repomap/images_update_test.gocmd/repomap/main.gocmd/repomap/main_test.gocmd/repomap/paths.gocmd/repomap/scan.godeps/chart_remote.godeps/collapse.godeps/collapse_test.godeps/common.godeps/compare.godeps/compare_pretty.godeps/compare_pretty_test.godeps/compare_scan.godeps/compare_scan_test.godeps/compare_test.godeps/discover.godeps/dockerfile.godeps/dockerfile_test.godeps/edgegraph.godeps/edgegraph_test.godeps/filter.godeps/go.godeps/go_graph.godeps/go_graph_test.godeps/go_test.godeps/gradle.godeps/gradle_tree.godeps/gradle_tree_test.godeps/helm_credentials.godeps/helm_credentials_test.godeps/image_base.godeps/maven.godeps/maven_tree.godeps/maven_tree_test.godeps/model.godeps/npm.godeps/npm_test.godeps/pnpm.godeps/pretty.godeps/pretty_tree.godeps/pretty_tree_test.godeps/remote_cache.godeps/remote_cache_test.godeps/remote_helm_client.godeps/remote_helm_client_test.godeps/resolve_remote.godeps/resolve_remote_test.godeps/runner.godeps/scan.godeps/scan_chart.godeps/scan_chart_test.godeps/scan_image.godeps/scan_test.godeps/update.godeps/update_apply.godeps/update_image.godeps/update_match.godeps/update_modes.godeps/update_modes_test.godeps/update_plan.godeps/update_prompt.godeps/update_prompt_test.godeps/update_resolve.godeps/update_stage.godeps/update_test.godeps/update_tree.godeps/update_tree_test.godeps/update_version.gogo.modimageupdate/chartref.goimageupdate/chartref_test.goimageupdate/discover.goimageupdate/discover_test.goimageupdate/extract.goimageupdate/labels.goimageupdate/resolver.goimageupdate/resolver_test.goimageupdate/sourceref.goimageupdate/sourceref_test.goimageupdate/target.goimageupdate/testdata/manifests/helmrelease-chartref-helmchart.yamlimageupdate/testdata/manifests/helmrelease-chartref-oci.yamltracked_yaml.go
💤 Files with no reviewable changes (1)
- cmd/repomap/images_update_test.go
| func resolveGoManifest(project Project, opts Options) (*Node, []Warning, error) { | ||
| file, err := loadGoModFile(project.Dir) | ||
| if err != nil { | ||
| return nil, nil, err | ||
| } | ||
| root := NewNode(ManagerGo, file.Module.Mod.Path, "") | ||
| root.Path = filepath.Join(project.Dir, "go.mod") | ||
| root.Source = "go.mod" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
golang.org/x/mod/modfile Parse Module field nil missing module directive
💡 Result:
In the golang.org/x/mod/modfile package, the Parse function does not return an error if the input go.mod file is missing a module directive [1]. Instead, it successfully returns a *modfile.File struct in which the Module field is nil [2][1]. This behavior can lead to runtime panics (e.g., null pointer dereferences) if code accessing the parsed File struct does not explicitly check whether Module is nil before attempting to access its fields [2][1]. Because modfile.Parse treats a missing module directive as a valid parsed state rather than a parse error, it is the responsibility of the consumer to validate the existence of the module declaration if required by their application logic [1]. For example, the Go toolchain's internal modload package includes explicit logic to check for a nil Module field after calling modfile.Parse and returns a custom error message when the declaration is missing [3][4]. When working with modfile.File, you should always perform a nil-check on the Module field before accessing it to ensure the go.mod file contains a valid module directive [1].
Citations:
- 1: mvdan/gofumpt@d802ec0
- 2: x/mod/modfile: document which fields in a parsed modfile may be nil golang/go#60898
- 3: https://go.dev/src/cmd/go/internal/modload/modfile.go
- 4: https://github.com/golang/go/blob/master/src/cmd/go/internal/modload/modfile.go
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)deps/(go\.(go|mod|sum)|go_graph\.go)|go.mod$|go.sum$' || true
echo "== locate relevant symbols =="
rg -n "func loadGoModFile|loadGoModFile|file\.Module|Module\.Mod\.Path|go.mod" deps -S
echo "== deps/go.go outline and relevant section =="
if [ -f deps/go.go ]; then
ast-grep outline deps/go.go --view compact || true
sed -n '1,80p' deps/go.go | cat -n
fi
echo "== deps/go_graph.go outline and relevant section =="
if [ -f deps/go_graph.go ]; then
ast-grep outline deps/go_graph.go --view compact || true
sed -n '1,60p' deps/go_graph.go | cat -n
sed -n '90,150p' deps/go_graph.go | cat -n
fiRepository: flanksource/repomap
Length of output: 14651
Validate go.mod modules before dereferencing Module. modfile.Parse does not return an error when go.mod has no module directive and can leave file.Module nil. Both deps/go.go and deps/go_graph.go call loadGoModFile and later dereference file.Module.Mod.Path; move the nil Module guard into loadGoModFile and return a descriptive error so malformed Go manifests fail as scan errors instead of panicking.
📍 Affects 2 files
deps/go.go#L11-L18(this comment)deps/go_graph.go#L20-L24
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/go.go` around lines 11 - 18, The loadGoModFile helper must validate that
the parsed manifest has a non-nil Module and return a descriptive error when the
module directive is missing, before callers dereference file.Module. Apply this
root-cause fix for both deps/go.go:11-18 and deps/go_graph.go:20-24; those call
sites require no direct changes beyond propagating the helper error.
| func (c *diskCache) cloneInto(ctx context.Context, url, ref, dir string) error { | ||
| tmp, err := os.MkdirTemp(c.root, "clone-*") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| defer func() { _ = os.RemoveAll(tmp) }() | ||
|
|
||
| args := []string{"clone", "--depth", "1", "--single-branch", url, tmp} | ||
| if ref != "" { | ||
| args = []string{"clone", "--depth", "1", "--single-branch", "--branch", ref, url, tmp} | ||
| } | ||
| if _, err := c.runner.Run(ctx, Command{Name: "git", Args: args}); err != nil { | ||
| // Ref may not be a branch/tag; fall back to the default branch. | ||
| if ref == "" { | ||
| return notFoundError{url} | ||
| } | ||
| if _, err2 := c.runner.Run(ctx, Command{Name: "git", Args: []string{"clone", "--depth", "1", url, tmp}}); err2 != nil { | ||
| return notFoundError{url} | ||
| } | ||
| } | ||
| if err := os.MkdirAll(filepath.Dir(dir), 0o755); err != nil { | ||
| return err | ||
| } | ||
| _ = os.RemoveAll(dir) | ||
| return os.Rename(tmp, dir) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Trace GitRepo callers and the image label that supplies the clone URL.
set -euo pipefail
echo "== GitRepo call sites =="
rg -nP --type=go -C 6 '\bGitRepo\s*\('
echo "== labelSource definition and readers =="
rg -nP --type=go -C 6 '\blabelSource\b'
echo "== base image resolution implementation =="
fd -t f 'image_base.go' --exec cat -n {}Repository: flanksource/repomap
Length of output: 182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files deps | sort | sed -n '1,160p'
echo "== remote_cache.go outline =="
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline deps/remote_cache.go --view expanded || true
fi
echo "== relevant source slices =="
for f in deps/remote_cache.go deps/resolve_remote.go; do
if [ -f "$f" ]; then
echo "--- $f ---"
wc -l "$f"
cat -n "$f" | sed -n '1,260p'
fi
done
echo "== dependency definitions including images and image labels =="
rg -n --type=go "type Image|ImageConfig|labelSource|labelBaseName|image.source|FROM |FROM.*image source|GitRepo|cloneInto|baseImages" deps -C 4 | sed -n '1,260p'Repository: flanksource/repomap
Length of output: 34349
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
root = Path("deps")
for p in root.glob("*.go"):
s = p.read_text()
for needle in ["GitRepo", "cloneInto", "ImageConfig", "labelSource", "baseImages"]:
if needle in s:
print(f"{p}: {needle} @ line(s) " + ") line(s) ".join(str(i+1) for i,line in enumerate(s.splitlines()) if needle in line))
print("\n== static argv construction probe ==")
src = Path("deps/remote_cache.go").read_text()
checks = {
"git_clone_args_with_url": "clone",
"git_clone_args_with_ref": "--branch",
"no_separator_before_url": '"--", url' in src or "'--', url" in src,
"env_allow_protocol": "GIT_ALLOW_PROTOCOL" in src,
}
for k,v in checks.items():
print(f"{k}: {v}")
PYRepository: flanksource/repomap
Length of output: 1195
🌐 Web query:
Git git-ext custom transport external commands URL documentation
💡 Result:
Git provides a mechanism for custom transport protocols through its remote helper architecture [1][2]. When Git encounters a repository URL with a prefix it does not natively recognize (e.g., <protocol>://), it attempts to locate and execute a program named git-remote-<protocol> in your system's PATH [1][2]. For simpler use cases, Git includes a built-in remote helper called git-remote-ext [3][4]. This tool allows you to pipe Git's native smart transport protocol through an arbitrary external command rather than writing a full-blown remote helper [3][4]. Usage of git-remote-ext: You can specify an external command using the ext:: prefix in your repository URL [5][3]: git remote add "ext::[ ...]" When invoked, Git executes the specified command and bridges the transport by connecting the command's standard input (stdin) and output (stdout) to Git's internal protocol stream [5][3]. The command is responsible for communicating with the remote server [5][3]. Key details: - Environment Variables: Git passes the requested service (e.g., git-upload-pack, git-receive-pack) to the external command via the GIT_EXT_SERVICE environment variable [5][3]. - Placeholders: Several placeholders can be used within the command string to pass repository information dynamically [3][6]: - %G: Replaced by the repository path [3][6]. - %S: Replaced by the service name [3][6]. - %V: Used to specify a virtual hostname [3][6]. - Security: Because git-remote-ext executes arbitrary shell commands, it presents a significant security risk [7][8]. It has been disabled by default in newer versions of Git (since version 2.38) and requires explicit configuration (e.g., protocol.ext.allow=always) to enable [8]. Vulnerabilities related to improper filtering of this transport (such as CVE-2026-62200) have highlighted the dangers of allowing this mechanism in environments where user-provided inputs might reach Git commands [8][9][10]. For more complex requirements, creating a dedicated git-remote-<protocol> helper program is the recommended approach, as it operates as a controlled, independent process that communicates with Git via a structured, line-based protocol on stdin/stdout [1][11][4].
Citations:
- 1: https://git-scm.com/docs/gitremote-helpers.html
- 2: https://stackoverflow.com/questions/17876132/can-i-create-a-custom-protocol-for-git
- 3: https://git-scm.com/docs/git-remote-ext
- 4: https://nesbitt.io/2026/03/18/git-remote-helpers.html
- 5: https://git-scm.tw/docs/git-remote-ext
- 6: https://git.github.io/htmldocs/git-remote-ext.html
- 7: https://www.codeant.ai/blogs/exploiting-git%E2%80%99s-ext-protocol-for-command-execution
- 8: https://github.com/nayakchinmohan/GHSA-9969-8g9h-rxwm
- 9: https://nvd.nist.gov/vuln/detail/CVE-2026-62200
- 10: https://notcve.org/cve/CVE-2026-62200
- 11: https://git.github.io/htmldocs/gitremote-helpers.html
🌐 Web query:
git clone git-upload-pack arbitrary command exploitation URL git clone
💡 Result:
The --upload-pack flag in git clone is a legitimate configuration option intended to specify the path to the git-upload-pack executable on a remote repository [1][2]. However, it is a well-documented vector for arbitrary command execution (ACE) or argument injection when misused, particularly in automated tools or libraries that wrap git commands [3][4][5]. The core issue arises because the --upload-pack option accepts an arbitrary string that is executed on the client-side system via a shell [6]. If an application allows untrusted user input to be passed as an argument to this flag, an attacker can supply malicious commands instead of a legitimate path [6]. Key security findings include: 1. Mechanism of Exploitation: When a git clone command is executed, the --upload-pack flag can be exploited to run arbitrary system commands by crafting input that breaks out of the intended argument structure [6]. For example, injecting command substitution payloads like '-u$({touch,/tmp/foo})' can lead to execution [6]. 2. Vulnerabilities in Wrappers: Multiple vulnerabilities have been identified in software libraries that wrap Git commands, such as GitPython and various npm packages [3][4][7][8]. These vulnerabilities often involve "argument injection," where the library fails to properly sanitize inputs, allowing user-controlled strings to reach the underlying git command as malicious arguments [3][7][8]. 3. Specific CVEs: - CVE-2026-42215: A vulnerability in GitPython where underscore-form keyword arguments (e.g., upload_pack) bypassed security guards intended to block the --upload-pack flag, leading to command injection [9][7]. - CVE-2022-25900 and CVE-2022-24437: Vulnerabilities in npm packages (git-clone and git-pull-or-clone) where inadequate neutralization of inputs allowed for argument injection via the --upload-pack feature [4][5][8][10]. To mitigate these risks, applications should avoid passing untrusted user input directly to Git commands, maintain strict allowlists for command-line arguments, and use libraries that explicitly sanitize or restrict access to dangerous options like --upload-pack [3][7]. Using higher-level, safe APIs that do not require constructing shell commands is recommended [4].
Citations:
- 1: https://git-scm.com/docs/git-upload-pack
- 2: https://git-scm.com/docs/git-fetch-pack
- 3: GHSA-r9mr-m37c-5fr3
- 4: GHSA-3x62-x456-q2vm
- 5: https://nvd.nist.gov/vuln/detail/CVE-2022-25900
- 6: https://sonarsource.github.io/argument-injection-vectors/binaries/git-clone/
- 7: GHSA-rpm5-65cw-6hj4
- 8: GHSA-8jmw-wjr8-2x66
- 9: https://nvd.nist.gov/vuln/detail/CVE-2026-42215
- 10: https://nvd.nist.gov/vuln/detail/CVE-2022-24437
Injection (CWE-88): Improper Neutralization of Argument Delimiters in a Command ('Argument Injection')
Reachability: External
Reachability path
● Entry
deps/update_test.go:583
TestUpdateRejectsUnsupportedManagers
│
▼
● Sink
deps/remote_cache.go
Reject unsafe repository URLs before calling git clone.
normalizeGitURL(source) accepts source values from org.opencontainers.image.source in scanned image metadata and passes them to cloneInto. cloneInto builds git clone argv directly, including ref after --branch, so a value beginning with - can change Git options and transport URLs can select Git protocols. Validate url with an allowlisted URL parser whose scheme/host matches only trusted repository sources, reject a ref starting with -, use -- before the position-only URL in all clone commands, and restrict allowed transports in the command environment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/remote_cache.go` around lines 171 - 196, Harden cloneInto before
invoking git: validate url with the repository-source allowlist used by
normalizeGitURL, permitting only trusted schemes and hosts; reject any ref
beginning with “-”. Add “--” before the positional URL in both clone argument
variants, and configure the git command environment to restrict transports to
the approved protocols.
Add cache warming for CI, sandbox, and air-gapped environments. Use real package managers to populate shared caches, optionally compile dependencies, and verify offline readiness while preserving partial results and actionable command errors.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
deps/cachewarm.go (1)
92-111: 🩺 Stability & Availability | 🔵 TrivialDocument that
CommandRunner.Runmust be concurrent-safe.
WarmCacheuses one injected runner acrosscacheWarmConcurrencygoroutines.ExecRunner{}is stateless, but callers can use recording or stateful test runners; add this to theCommandRunnerinterface contract or add tests proving injected runners handle concurrentRuncalls safely.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deps/cachewarm.go` around lines 92 - 111, Update the CommandRunner interface contract to explicitly require that Run supports concurrent calls from multiple goroutines, reflecting WarmCache’s shared runner usage under cacheWarmConcurrency. Document this concurrency guarantee near the interface and preserve existing runner implementations; alternatively, add focused tests demonstrating injected stateful or recording runners safely handle concurrent Run calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deps/cachewarm.go`:
- Around line 264-285: Update walkWarmed to increment count and capture the
matching version when a distinct node is popped, after the nil/seen check and
before traversing its children; remove the child-loop accounting so shared
dependencies are counted only once.
In `@README.md`:
- Around line 114-123: Update the README paragraph describing `--build` to
explicitly state that Go builds persist compiled artifacts in `GOCACHE`, in
addition to dependency sources in `GOMODCACHE`; keep the existing npm and pnpm
cache descriptions unchanged.
---
Nitpick comments:
In `@deps/cachewarm.go`:
- Around line 92-111: Update the CommandRunner interface contract to explicitly
require that Run supports concurrent calls from multiple goroutines, reflecting
WarmCache’s shared runner usage under cacheWarmConcurrency. Document this
concurrency guarantee near the interface and preserve existing runner
implementations; alternatively, add focused tests demonstrating injected
stateful or recording runners safely handle concurrent Run calls.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 937fa7a5-c5c1-4865-958c-237431bb721e
📒 Files selected for processing (20)
README.mdcmd/repomap/cache_warm.gocmd/repomap/cache_warm_test.godeps/cachewarm.godeps/cachewarm_plan.godeps/cachewarm_test.godeps/manager/gomod/warm.godeps/manager/gomod/warm_test.godeps/manager/node/warm.godeps/manager/node/warm_test.godeps/manager/npm/warm.godeps/manager/npm/warm_test.godeps/manager/pnpm/warm.godeps/manager/pnpm/warm_test.godeps/manifest/command.godeps/manifest/manager.godeps/manifest/warm.godeps/model.godeps/runner.godeps/update_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- deps/update_test.go
| func walkWarmed(root *Node, name string) (count int, version string) { | ||
| seen := map[*Node]bool{} | ||
| stack := []*Node{root} | ||
| for len(stack) > 0 { | ||
| node := stack[len(stack)-1] | ||
| stack = stack[:len(stack)-1] | ||
| if node == nil || seen[node] { | ||
| continue | ||
| } | ||
| seen[node] = true | ||
| for _, child := range node.Children { | ||
| if child.Version != "" { | ||
| count++ | ||
| if version == "" && child.Name == name { | ||
| version = child.Version | ||
| } | ||
| } | ||
| stack = append(stack, child) | ||
| } | ||
| } | ||
| return count, version | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Count each package once.
The counter increments for every edge, not for every distinct node. seen deduplicates the traversal but not the count. A dependency reachable from two parents is counted twice, so Packages overstates the closure for graphs that share transitive dependencies. Move the accounting to the pop site, after the seen check.
🐛 Proposed fix to count distinct nodes
func walkWarmed(root *Node, name string) (count int, version string) {
seen := map[*Node]bool{}
stack := []*Node{root}
for len(stack) > 0 {
node := stack[len(stack)-1]
stack = stack[:len(stack)-1]
if node == nil || seen[node] {
continue
}
seen[node] = true
+ if node != root && node.Version != "" {
+ count++
+ if version == "" && node.Name == name {
+ version = node.Version
+ }
+ }
for _, child := range node.Children {
- if child.Version != "" {
- count++
- if version == "" && child.Name == name {
- version = child.Version
- }
- }
stack = append(stack, child)
}
}
return count, version
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func walkWarmed(root *Node, name string) (count int, version string) { | |
| seen := map[*Node]bool{} | |
| stack := []*Node{root} | |
| for len(stack) > 0 { | |
| node := stack[len(stack)-1] | |
| stack = stack[:len(stack)-1] | |
| if node == nil || seen[node] { | |
| continue | |
| } | |
| seen[node] = true | |
| for _, child := range node.Children { | |
| if child.Version != "" { | |
| count++ | |
| if version == "" && child.Name == name { | |
| version = child.Version | |
| } | |
| } | |
| stack = append(stack, child) | |
| } | |
| } | |
| return count, version | |
| } | |
| func walkWarmed(root *Node, name string) (count int, version string) { | |
| seen := map[*Node]bool{} | |
| stack := []*Node{root} | |
| for len(stack) > 0 { | |
| node := stack[len(stack)-1] | |
| stack = stack[:len(stack)-1] | |
| if node == nil || seen[node] { | |
| continue | |
| } | |
| seen[node] = true | |
| if node != root && node.Version != "" { | |
| count++ | |
| if version == "" && node.Name == name { | |
| version = node.Version | |
| } | |
| } | |
| for _, child := range node.Children { | |
| stack = append(stack, child) | |
| } | |
| } | |
| return count, version | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@deps/cachewarm.go` around lines 264 - 285, Update walkWarmed to increment
count and capture the matching version when a distinct node is popped, after the
nil/seen check and before traversing its children; remove the child-loop
accounting so shared dependencies are counted only once.
| For each `name@version` spec, repomap creates a throwaway single-dependency | ||
| project in a temporary directory, drives the real package manager to download the | ||
| closure into the machine's shared cache, then deletes the project. Nothing in the | ||
| working tree is touched; what persists is the warmed cache (`GOMODCACHE`, the pnpm | ||
| store, or the npm cache). Omit the version to take whatever the manager considers | ||
| current — the concrete resolved version is reported back, including Go | ||
| pseudo-versions. | ||
|
|
||
| `--build` goes further than downloading. For Go it compiles every package in the | ||
| module so `GOCACHE` holds build artifacts and not just source. For npm and pnpm it |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document GOCACHE persistence for Go builds.
This paragraph lists only dependency caches as persistent outputs. Lines 122-123 state that Go --build also persists compiled artifacts in GOCACHE. Clarify this distinction so the documented cache results match the --build behavior.
Proposed wording
- project. Nothing in the working tree is touched; what persists is the warmed cache (`GOMODCACHE`, the pnpm store, or the npm cache).
+ project. Nothing in the working tree is touched; the warmed dependency cache persists in `GOMODCACHE`, the pnpm store, or the npm cache. With Go `--build`, compiled artifacts also persist in `GOCACHE`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| For each `name@version` spec, repomap creates a throwaway single-dependency | |
| project in a temporary directory, drives the real package manager to download the | |
| closure into the machine's shared cache, then deletes the project. Nothing in the | |
| working tree is touched; what persists is the warmed cache (`GOMODCACHE`, the pnpm | |
| store, or the npm cache). Omit the version to take whatever the manager considers | |
| current — the concrete resolved version is reported back, including Go | |
| pseudo-versions. | |
| `--build` goes further than downloading. For Go it compiles every package in the | |
| module so `GOCACHE` holds build artifacts and not just source. For npm and pnpm it | |
| For each `name@version` spec, repomap creates a throwaway single-dependency | |
| project in a temporary directory, drives the real package manager to download the | |
| closure into the machine's shared cache, then deletes the project. Nothing in the | |
| working tree is touched; the warmed dependency cache persists in `GOMODCACHE`, the | |
| pnpm store, or the npm cache. With Go `--build`, compiled artifacts also persist | |
| in `GOCACHE`. Omit the version to take whatever the manager considers | |
| current — the concrete resolved version is reported back, including Go | |
| pseudo-versions. | |
| `--build` goes further than downloading. For Go it compiles every package in the | |
| module so `GOCACHE` holds build artifacts and not just source. For npm and pnpm it |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 114 - 123, Update the README paragraph describing
`--build` to explicitly state that Go builds persist compiled artifacts in
`GOCACHE`, in addition to dependency sources in `GOMODCACHE`; keep the existing
npm and pnpm cache descriptions unchanged.
1 participant
What
deps updateand combine them with positional patterns.Notes
UpdateOptions.ExpressiontoFiltersand updateVersionSelectorto receiveUpdateVersionPrompt.Summary by CodeRabbit
New Features
deps difffor comparing dependency changes across Git revisions.cache-warmto prepare and verify dependency caches, including offline workflows.Documentation
Bug Fixes