Conversation
…ing from top to bottom the semversions
There was a problem hiding this comment.
Pull request overview
This PR adds a prototype “quick fix” workflow under cmd/devguard-cli/test/ to fetch package metadata from registries (npm/crates) and attempt to validate whether updating a dependency chain results in a vulnerable package version being at/above a fixed version.
Changes:
- Add npm registry response type definitions for JSON unmarshalling.
- Add registry helper functions (npm/crates) and a quickfix CLI that walks a dependency chain.
- Add semver parsing/recommendation logic to select newer versions within the same major band.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 13 comments.
| File | Description |
|---|---|
cmd/devguard-cli/test/types.go |
Introduces structs for npm registry JSON responses. |
cmd/devguard-cli/test/quickfix.go |
Implements the dependency-chain “quick fix” logic and a main() driver. |
cmd/devguard-cli/test/package_manager_functions.go |
Adds registry request helpers for npm/crates and a version existence check. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 23 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…current version, else it should fail
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 19 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cmd/devguard-cli/test/quickfix.go
Outdated
| func parsePurl(purl string) (string, string, error) { | ||
| // Format: pkg:npm/package-name@version or pkg:npm/@scoped/package@version | ||
| // Note: version can be empty string to indicate "all versions" (see RegistryRequest) | ||
| input, err := packageurl.FromString(purl) | ||
| if err != nil { | ||
| return "", "", fmt.Errorf("invalid purl format: %w", err) | ||
| } | ||
|
|
||
| pkgName := input.Name | ||
| if input.Namespace != "" { | ||
| pkgName = input.Namespace + "/" + input.Name | ||
| } | ||
|
|
||
| version := input.Version | ||
|
|
||
| return pkgName, version, nil | ||
| } |
There was a problem hiding this comment.
The parsePurl function only returns the package name and version, but doesn't return the package type (npm, cargo, etc.) which is available in the purl. This type information is needed to properly determine which package manager/registry to query. The function should return a third value (input.Type) or the entire PackageURL struct so callers can determine the correct package manager.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (10)
cmd/devguard-cli/test/quickfix.go:243
- The comment block above resolveBestVersion says OR expressions are "not implemented yet", but the function now implements "||" handling below. Please update/remove the stale comment to avoid misleading future readers.
// Handle OR expressions - not implemented yet, return error
// if strings.Contains(versionSpec, "||") {
// return "", fmt.Errorf("OR expressions (||) not yet supported: %s", versionSpec)
// }
cmd/devguard-cli/test/quickfix.go:297
- There are remaining fmt.Println debug statements (printing parts) inside resolveBestVersion’s OR-handling loop. These will pollute stdout and make the function unsuitable for library use/tests. Remove the prints or replace them with structured debug logging behind a flag.
parts := strings.Split(orBaseVersion, ".")
fmt.Println(parts)
// 14.0 is not getting caught by semver.isValid
cmd/devguard-cli/test/quickfix.go:121
- parseVersion writes an "ERROR" line to stderr on parse failure. This helper is used in sorting/comparisons and will create noisy output in tests and CLI runs; it also silently converts parse failures to 0.0.0. Prefer returning an error to the caller, or (since callers already semver-validate) remove the stderr logging and handle invalid input deterministically.
if _, err := fmt.Sscanf(cleanVersion, "%d.%d.%d", &result[0], &result[1], &result[2]); err != nil {
fmt.Fprintf(os.Stderr, "ERROR: Failed to parse version %q: %v (this indicates a validation bypass)\n", version, err)
return [3]int{0, 0, 0}
}
cmd/devguard-cli/test/quickfix.go:170
- splitOrExpression has a fmt.Println(result) side effect. This will spam stdout during tests/usage and makes the helper non-deterministic. Remove the print or gate it behind an explicit debug logger/flag.
fmt.Println(result)
return result
cmd/devguard-cli/test/quickfix.go:457
- getVersion supports both npm and crates, but fetchPackageMetadata always decodes responses into NPMResponse. If pkgManager is ever "crates", JSON decoding will be wrong. Either restrict this helper to npm/node explicitly, or introduce per-registry response types so decoding matches the selected registry.
var npmResp NPMResponse
if err := json.NewDecoder(resp.Body).Decode(&npmResp); err != nil {
return nil, fmt.Errorf("error decoding JSON for %s@%s: %w", dep, version, err)
cmd/devguard-cli/test/quickfix.go:467
- This directory defines a package main with its own main() entrypoint under cmd/devguard-cli/test. If CI/dev scripts ever build/install ./cmd/... this will produce an extra binary (named "test") and ship hard-coded network behavior. If this is only a local playground, consider moving it out of cmd/ or adding a build tag to exclude it from normal builds.
func main() {
purls := []string{
"pkg:npm/@sentry/nextjs@9.38.0",
/*
nextjs@9.39.0 ---> Ist abhängig von react@6.28.0
cmd/devguard-cli/test/types.go:94
- Type name "Signatures" is plural but represents a single signature object. For Go naming clarity, prefer a singular type name (e.g. Signature) and keep the slice field plural (e.g. Signatures []Signature).
type Signatures struct {
Sig string `json:"sig"`
KeyID string `json:"keyid"`
}
cmd/devguard-cli/test/quickfix_test.go:5
- The new test file uses an Apache-2.0 license header, while other files under cmd/devguard-cli use the AGPL header (e.g. cmd/devguard-cli/commands/root.go). If this file isn’t copied from an Apache-licensed upstream source, update the header to match the rest of the devguard-cli code to avoid license inconsistencies.
// Copyright 2026 larshermges
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
cmd/devguard-cli/test/package_manager_functions.go:41
- The comment says "remove quotes if present" but the code trims '/' characters from the version. Either adjust the comment to match what’s happening or trim the actual characters you intend to normalize (and consider validating the version path segment).
normalizedVersion := strings.Trim(pkg.Version, "/") // remove quotes if present
if pkg.Version != "" {
cmd/devguard-cli/test/quickfix.go:470
- There’s an inline multi-line comment in German inside main(). This makes the codebase harder to maintain for non-German speakers and reads like a scratch note. Prefer converting this to an English comment with actionable context, or move the reasoning to an issue/PR description.
/*
nextjs@9.39.0 ---> Ist abhängig von react@6.28.0
packages[i+1].version = 6.28.0
1. Erneut getRecommendedVersions für react@6.28.0??? Aber ist doch fixed von nextjs
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (8)
cmd/devguard-cli/test/package_manager_functions.go:40
strings.Trim(pkg.Version, "/")does not "remove quotes" (it removes/characters). If the intent is to normalize user input, use a quote/whitespace trim that matches the comment (or update the comment so it reflects the actual behavior).
normalizedVersion := strings.Trim(pkg.Version, "/") // remove quotes if present
cmd/devguard-cli/test/package_manager_functions.go:3
- New file header deviates from the standard header used elsewhere in
cmd/devguard-cli(e.g.,// Copyright (C) 2026 l3montree GmbH). To keep licensing/copyright notices consistent, align this header with the existing convention.
// Copyright 2026 lars hermges @ l3montree GmbH
// This program is free software: you can redistribute it and/or modify
cmd/devguard-cli/test/quickfix.go:258
- This comment says OR expressions are "not implemented yet", but the function now implements
||handling below. This is misleading for future readers; update or remove the comment to match the current behavior.
// Handle OR expressions - not implemented yet, return error
// if strings.Contains(versionSpec, "||") {
// return "", fmt.Errorf("OR expressions (||) not yet supported: %s", versionSpec)
// }
cmd/devguard-cli/test/quickfix.go:188
>=and>are implemented as "same major version" comparisons. That differs from typical semver range semantics (where>=1.2.3can match2.0.0), and the inline comments don’t mention this restriction. Either document this as an intentional "same-major" policy (including in the rangeType docs), or adjust the logic to follow standard operator semantics.
case ">=":
// Greater than or equal: same major version, >= base
return versionParts[0] == baseParts[0] && semver.Compare("v"+version, "v"+baseVersion) >= 0
case ">":
// Greater than: same major version, > base
return versionParts[0] == baseParts[0] && semver.Compare("v"+version, "v"+baseVersion) > 0
.vscode/launch.json:53
- The new launch configuration block is indented differently from the surrounding entries and has an empty multiline
argsarray, which makes the JSONC harder to read/maintain. Consider matching the existing formatting (indentation and"args": []style) used by the other configurations in this file.
{
"name": "Launch DepTreeWalk",
"type": "go",
"request": "launch",
"cwd": "${workspaceRoot}",
"mode": "auto",
"program": "${workspaceRoot}/cmd/devguard-cli/test",
"args": [
],
},
cmd/devguard-cli/test/package_manager_functions.go:69
- Similar to the NPM call,
pkg.Dependency/pkg.Versionare concatenated into the URL path without escaping. While crate names are usually simple, this still permits malformed input (extra slashes,..) to change the requested path. Consider URL-escaping these path segments before building the request URL.
if pkg.Version != "" {
req, err = httpClient.Get("https://crates.io/api/v1/crates/" + pkg.Dependency + "/" + pkg.Version)
} else {
req, err = httpClient.Get("https://crates.io/api/v1/crates/" + pkg.Dependency)
}
cmd/devguard-cli/test/quickfix.go:3
- The file header differs from the standard AGPL header format used in other
cmd/devguard-cliGo files (e.g., thecommandspackage). Aligning the copyright line/format avoids inconsistency in licensing notices across the repo.
// Copyright 2026 lars hermges @ l3montree GmbH
// This program is free software: you can redistribute it and/or modify
cmd/devguard-cli/test/types.go:95
- The type name
Signaturesis plural, but it represents a single signature, and it results in the awkward fieldSignatures []Signatures. Renaming the struct toSignature(and keeping the field nameSignatures) would make the API clearer.
type Dist struct {
Shasum string `json:"shasum"`
Tarball string `json:"tarball"`
Integrity string `json:"integrity"`
Signatures []Signatures `json:"signatures"`
}
type Repository struct {
URL string `json:"url"`
Type string `json:"type"`
}
type Signatures struct {
Sig string `json:"sig"`
KeyID string `json:"keyid"`
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 participant
No description provided.