From 0e3b7444f965e114831cfb8de671b677142c0aa1 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Tue, 15 Sep 2026 19:17:23 +0100 Subject: [PATCH 1/3] Add version suffix to gomod and cargo add AddOptions.Version was silently dropped for gomod and cargo because their add.args declared no version arg. Add it with suffix "@" to match npm.yaml, so mgr.Add builds `go get pkg@v` and `cargo add pkg@v`. Fixes #38 --- definitions/cargo.yaml | 1 + definitions/gomod.yaml | 1 + generic_manager_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ translator_test.go | 28 ++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+) diff --git a/definitions/cargo.yaml b/definitions/cargo.yaml index 82a8e7e..45920d2 100644 --- a/definitions/cargo.yaml +++ b/definitions/cargo.yaml @@ -42,6 +42,7 @@ commands: args: # cargo add supports package@version syntax package: {position: 0, required: true, validate: cargo_crate} + version: {position: 0, suffix: "@", required: false} flags: dev: [--dev] build: [--build] diff --git a/definitions/gomod.yaml b/definitions/gomod.yaml index 3f21c77..72e9e72 100644 --- a/definitions/gomod.yaml +++ b/definitions/gomod.yaml @@ -41,6 +41,7 @@ commands: base: [get] args: package: {position: 0, required: true, validate: go_module} + version: {position: 0, suffix: "@", required: false} flags: # -t includes test dependencies test: [-t] diff --git a/generic_manager_test.go b/generic_manager_test.go index bf7b973..e5b5241 100644 --- a/generic_manager_test.go +++ b/generic_manager_test.go @@ -20,6 +20,47 @@ func newTestManager(def *definitions.Definition, runner *MockRunner) *GenericMan } } +func embeddedDef(t *testing.T, name string) *definitions.Definition { + t.Helper() + defs, err := definitions.LoadEmbedded() + if err != nil { + t.Fatalf("LoadEmbedded: %v", err) + } + for _, d := range defs { + if d.Name == name { + return d + } + } + t.Fatalf("no embedded definition %q", name) + return nil +} + +func TestGenericManager_Add_GomodVersion(t *testing.T) { + runner := NewMockRunner() + mgr := newTestManager(embeddedDef(t, "gomod"), runner) + _, err := mgr.Add(context.Background(), "github.com/pkg/errors", AddOptions{Version: "v0.9.1"}) + if err != nil { + t.Fatalf("Add: %v", err) + } + want := []string{"go", "get", "github.com/pkg/errors@v0.9.1"} + if !slicesEqual(runner.Captured[0], want) { + t.Errorf("got %v, want %v", runner.Captured[0], want) + } +} + +func TestGenericManager_Add_CargoVersion(t *testing.T) { + runner := NewMockRunner() + mgr := newTestManager(embeddedDef(t, "cargo"), runner) + _, err := mgr.Add(context.Background(), "serde", AddOptions{Version: "1.0.219"}) + if err != nil { + t.Fatalf("Add: %v", err) + } + want := []string{"cargo", "add", "serde@1.0.219"} + if !slicesEqual(runner.Captured[0], want) { + t.Errorf("got %v, want %v", runner.Captured[0], want) + } +} + func TestGenericManager_Path_Raw(t *testing.T) { def := &definitions.Definition{ Name: "testpkg", diff --git a/translator_test.go b/translator_test.go index 2a2fd83..6eb2db0 100644 --- a/translator_test.go +++ b/translator_test.go @@ -315,6 +315,20 @@ func TestCargoAdd(t *testing.T) { } } +func TestCargoAddVersion(t *testing.T) { + tr := loadTranslator(t) + cmd, err := tr.BuildCommand("cargo", "add", CommandInput{ + Args: map[string]string{"package": "serde", "version": "1.0.219"}, + }) + if err != nil { + t.Fatalf("BuildCommand failed: %v", err) + } + expected := []string{"cargo", "add", "serde@1.0.219"} + if !reflect.DeepEqual(cmd, expected) { + t.Errorf("got %v, want %v", cmd, expected) + } +} + func TestCargoAddDev(t *testing.T) { tr := loadTranslator(t) cmd, err := tr.BuildCommand("cargo", "add", CommandInput{ @@ -398,6 +412,20 @@ func TestGomodAdd(t *testing.T) { } } +func TestGomodAddVersion(t *testing.T) { + tr := loadTranslator(t) + cmd, err := tr.BuildCommand("gomod", "add", CommandInput{ + Args: map[string]string{"package": "github.com/pkg/errors", "version": "v0.9.1"}, + }) + if err != nil { + t.Fatalf("BuildCommand failed: %v", err) + } + expected := []string{"go", "get", "github.com/pkg/errors@v0.9.1"} + if !reflect.DeepEqual(cmd, expected) { + t.Errorf("got %v, want %v", cmd, expected) + } +} + func TestGomodAddChain(t *testing.T) { tr := loadTranslator(t) cmds, err := tr.BuildCommands("gomod", "add", CommandInput{ From d963aa980b6a9de8fc885fd19593976b7a10fac7 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Tue, 15 Sep 2026 19:49:35 +0100 Subject: [PATCH 2/3] Track package arg index for version suffix applyVersionSuffix found the package by string-matching argv, so a package name equal to the binary or a base subcommand (a crate named "add" or "cargo", an npm package named "install") had the wrong token rewritten. Record the index where the package positional is appended and rewrite that element directly. --- translator.go | 42 +++++++++++++++++++++++++----------------- translator_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/translator.go b/translator.go index 54cfc86..4b62005 100644 --- a/translator.go +++ b/translator.go @@ -93,15 +93,17 @@ func (t *Translator) buildSingleCommand(binary string, cmd definitions.Command, baseOverrideUsed := t.applyBaseOverrides(&args, cmd, input) - packageVal := input.Args["package"] - sortedArgs := t.sortArgs(cmd) + packageIdx := -1 for _, entry := range sortedArgs { - val, err := t.processArg(entry.name, entry.argDef, input, &args) + val, at, err := t.processArg(entry.name, entry.argDef, input, &args) if err != nil { return nil, err } + if entry.name == "package" { + packageIdx = at + } if val == "" { continue } @@ -110,7 +112,7 @@ func (t *Translator) buildSingleCommand(binary string, cmd definitions.Command, } } - t.applyVersionSuffix(&args, cmd, input, packageVal) + t.applyVersionSuffix(&args, cmd, input, packageIdx) if !suppressDefaultFlags { args = append(args, cmd.DefaultFlags...) @@ -158,25 +160,29 @@ func (t *Translator) sortArgs(cmd definitions.Command) []argEntry { return sorted } -func (t *Translator) processArg(name string, argDef definitions.Arg, input CommandInput, args *[]string) (string, error) { +// processArg appends the argument to args and returns the value, the index +// at which the value was appended (or -1 when nothing was appended or the +// value is not a standalone positional), and any validation error. +func (t *Translator) processArg(name string, argDef definitions.Arg, input CommandInput, args *[]string) (string, int, error) { val, provided := input.Args[name] if !provided { if argDef.Required && !argDef.ExtractionOnly { - return "", ErrMissingArgument{Argument: name} + return "", -1, ErrMissingArgument{Argument: name} } - return "", nil + return "", -1, nil } if argDef.ExtractionOnly { - return "", nil + return "", -1, nil } if argDef.Validate != "" { if err := t.validate(argDef.Validate, val); err != nil { - return "", err + return "", -1, err } } + at := -1 switch { case argDef.Flag != "": *args = append(*args, argDef.Flag, val) @@ -186,12 +192,19 @@ func (t *Translator) processArg(name string, argDef definitions.Arg, input Comma // Handled in applyVersionSuffix default: *args = append(*args, val) + at = len(*args) - 1 } - return val, nil + return val, at, nil } -func (t *Translator) applyVersionSuffix(args *[]string, cmd definitions.Command, input CommandInput, packageVal string) { +// applyVersionSuffix appends the version to the package argument in place. +// packageIdx is the index into args where the package positional was +// written; a negative value means no package positional was appended. +func (t *Translator) applyVersionSuffix(args *[]string, cmd definitions.Command, input CommandInput, packageIdx int) { + if packageIdx < 0 || packageIdx >= len(*args) { + return + } versionDef, hasVersion := cmd.Args["version"] if !hasVersion || versionDef.Suffix == "" { return @@ -200,12 +213,7 @@ func (t *Translator) applyVersionSuffix(args *[]string, cmd definitions.Command, if !hasVersionVal { return } - for i, a := range *args { - if a == packageVal { - (*args)[i] = a + versionDef.Suffix + version - break - } - } + (*args)[packageIdx] += versionDef.Suffix + version } func (t *Translator) applyUserFlags(args *[]string, cmd definitions.Command, input CommandInput, baseOverrideUsed string) { diff --git a/translator_test.go b/translator_test.go index 6eb2db0..c44634d 100644 --- a/translator_test.go +++ b/translator_test.go @@ -329,6 +329,36 @@ func TestCargoAddVersion(t *testing.T) { } } +func TestVersionSuffixPackageNameCollision(t *testing.T) { + tr := loadTranslator(t) + cases := []struct { + manager string + pkg string + version string + want []string + }{ + // crate name equal to the base subcommand + {"cargo", "add", "1.0", []string{"cargo", "add", "add@1.0"}}, + // crate name equal to the binary + {"cargo", "cargo", "1.0", []string{"cargo", "add", "cargo@1.0"}}, + // npm package name equal to the base subcommand + {"npm", "install", "2.0", []string{"npm", "install", "install@2.0"}}, + // go module path equal to the base subcommand + {"gomod", "get", "v1.0.0", []string{"go", "get", "get@v1.0.0"}}, + } + for _, tc := range cases { + got, err := tr.BuildCommand(tc.manager, "add", CommandInput{ + Args: map[string]string{"package": tc.pkg, "version": tc.version}, + }) + if err != nil { + t.Fatalf("%s %s: %v", tc.manager, tc.pkg, err) + } + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("%s %s: got %v, want %v", tc.manager, tc.pkg, got, tc.want) + } + } +} + func TestCargoAddDev(t *testing.T) { tr := loadTranslator(t) cmd, err := tr.BuildCommand("cargo", "add", CommandInput{ From 5ddc92a0183f1676927b84286ee79224cd9fcf3d Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Tue, 15 Sep 2026 21:26:26 +0100 Subject: [PATCH 3/3] Track flagged-package index; strip version in blocklist match processArg left the index at -1 for a package arg declared with Flag, so a definition combining a flagged package with a suffix version lost the version. Set the index to the appended value in that branch too. PackageBlocklistPolicy exact-matched op.Packages entries, so a versioned positional such as `go get pkg@v1.2.3` bypassed a blocklist keyed on the bare name. Check the bare form (before the last @) as well, preserving the leading @ of npm scoped names. --- policy.go | 35 +++++++++++++++++++++++++++-------- policy_test.go | 33 +++++++++++++++++++++++++++++++++ translator.go | 1 + translator_test.go | 27 +++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 8 deletions(-) diff --git a/policy.go b/policy.go index d94a038..8ea2c8b 100644 --- a/policy.go +++ b/policy.go @@ -255,15 +255,34 @@ func (PackageBlocklistPolicy) Name() string { return "package-blocklist" } func (p PackageBlocklistPolicy) Check(ctx context.Context, op *PolicyOperation) (*PolicyResult, error) { for _, pkg := range op.Packages { - if reason, blocked := p.Blocked[pkg]; blocked { - return &PolicyResult{ - Allowed: false, - Reason: reason, - Metadata: map[string]any{ - "blocked_package": pkg, - }, - }, nil + if r := p.match(pkg); r != nil { + return r, nil } } return &PolicyResult{Allowed: true}, nil } + +// match checks pkg against the blocklist, both as given and with any +// trailing @version stripped so that a versioned add such as +// `go get example.com/foo@v1.2.3` or `npm install @scope/name@1.0.0` +// is caught by an entry keyed on the bare package name. +func (p PackageBlocklistPolicy) match(pkg string) *PolicyResult { + if reason, blocked := p.Blocked[pkg]; blocked { + return blockedResult(pkg, reason) + } + if i := strings.LastIndex(pkg, "@"); i > 0 { + bare := pkg[:i] + if reason, blocked := p.Blocked[bare]; blocked { + return blockedResult(bare, reason) + } + } + return nil +} + +func blockedResult(pkg, reason string) *PolicyResult { + return &PolicyResult{ + Allowed: false, + Reason: reason, + Metadata: map[string]any{"blocked_package": pkg}, + } +} diff --git a/policy_test.go b/policy_test.go index 983fbc6..381a53d 100644 --- a/policy_test.go +++ b/policy_test.go @@ -153,6 +153,7 @@ func TestPackageBlocklistPolicy(t *testing.T) { {"blocked package", []string{"evil-package"}, false}, {"mixed packages", []string{"lodash", "deprecated-lib"}, false}, {"empty packages", []string{}, true}, + {"versioned blocked", []string{"evil-package@1.2.3"}, false}, } for _, tt := range tests { @@ -169,6 +170,32 @@ func TestPackageBlocklistPolicy(t *testing.T) { } } +func TestPackageBlocklistScopedAndVersioned(t *testing.T) { + policy := PackageBlocklistPolicy{ + Blocked: map[string]string{ + "@scope/name": "scoped bare", + "github.com/org/mod": "go module", + }, + } + cases := []struct { + pkg string + allowed bool + }{ + {"@scope/name", false}, + {"@scope/name@7.0.0", false}, + {"@scope/other", true}, + {"github.com/org/mod@v1.2.3", false}, + {"github.com/org/mod", false}, + {"github.com/org/other@v1.0.0", true}, + } + for _, tc := range cases { + res, _ := policy.Check(context.Background(), &PolicyOperation{Packages: []string{tc.pkg}}) + if res.Allowed != tc.allowed { + t.Errorf("%s: allowed=%v, want %v", tc.pkg, res.Allowed, tc.allowed) + } + } +} + func TestPackageBlocklistViaRunner(t *testing.T) { mock := NewMockRunner() policy := PackageBlocklistPolicy{ @@ -194,6 +221,12 @@ func TestPackageBlocklistViaRunner(t *testing.T) { if err != nil { t.Fatalf("allowed package should pass: %v", err) } + + // A versioned form of a blocked package must also be denied. + _, err = pr.Run(context.Background(), "/tmp", "go", "get", "evil-package@v1.2.3") + if err == nil { + t.Fatal("expected blocklist policy to deny versioned package") + } } func TestPolicyRunnerWithContext(t *testing.T) { diff --git a/translator.go b/translator.go index 4b62005..5d73fa0 100644 --- a/translator.go +++ b/translator.go @@ -186,6 +186,7 @@ func (t *Translator) processArg(name string, argDef definitions.Arg, input Comma switch { case argDef.Flag != "": *args = append(*args, argDef.Flag, val) + at = len(*args) - 1 case argDef.FixedSuffix != "": *args = append(*args, val+argDef.FixedSuffix) case argDef.Suffix != "" && name == "version": diff --git a/translator_test.go b/translator_test.go index c44634d..d0545fb 100644 --- a/translator_test.go +++ b/translator_test.go @@ -329,6 +329,33 @@ func TestCargoAddVersion(t *testing.T) { } } +func TestVersionSuffixOnFlaggedPackage(t *testing.T) { + tr := NewTranslator() + tr.Register(&definitions.Definition{ + Name: "flagpkg", + Binary: "tool", + Commands: map[string]definitions.Command{ + "add": { + Base: []string{"add"}, + Args: map[string]definitions.Arg{ + "package": {Flag: "--package", Required: true}, + "version": {Suffix: "@"}, + }, + }, + }, + }) + got, err := tr.BuildCommand("flagpkg", "add", CommandInput{ + Args: map[string]string{"package": "foo", "version": "1.0"}, + }) + if err != nil { + t.Fatalf("BuildCommand: %v", err) + } + want := []string{"tool", "add", "--package", "foo@1.0"} + if !reflect.DeepEqual(got, want) { + t.Errorf("got %v, want %v", got, want) + } +} + func TestVersionSuffixPackageNameCollision(t *testing.T) { tr := loadTranslator(t) cases := []struct {