feat(config): add cobra-probe-enabled toggle option and safeguard cobra probing (#133)
currently, every command that doesn't have a completion spec is assumed to be a cobra cli and it gets ran with a `__complete` argument. for non cobra clis this can cause problems if those non-cobra clis have sideeffects even when ran with the `__complete` argument, and there is no way to disable this. one of these problems was solved at #111, but a full solution would probably require actual sandboxing which is i think is an overkill just for attempting to get suggestions for unrecognized commands. instead of sandboxing, this pr adds a config option for allowing only selected commands to be probed and disallowing the rest. currently the default is still allowing all (`["*"]`) so default behavior is the same, but it might be better to just have a long list of known cobra clis as the default instead. here's a demo of a side effect caused by the probing, in this case "deleting" a file with rmtrash without actually trying to run the command: https://github.com/user-attachments/assets/5256d363-5291-42e8-bb4f-cf7467927246 --------- Co-authored-by: shemishtamesh <shemishtamail@gmail.com>
This commit is contained in:
co-authored by
shemishtamesh
parent
0f173f78dc
commit
89fcd6f830
@@ -301,6 +301,7 @@ expand-alias = true # expand aliases before matching
|
|||||||
auto-execute = false # run suggestion immediately instead of inserting it
|
auto-execute = false # run suggestion immediately instead of inserting it
|
||||||
atuin-history = 0 # 0 = shell history, 1 = atuin, 2 = both
|
atuin-history = 0 # 0 = shell history, 1 = atuin, 2 = both
|
||||||
atuin-db-path = "" # path to atuin's history.db, empty = use default
|
atuin-db-path = "" # path to atuin's history.db, empty = use default
|
||||||
|
cobra-probe-enabled = true # fall back to probing cobra binaries for completions
|
||||||
|
|
||||||
[ui]
|
[ui]
|
||||||
style = "modern" # "modern" or "classic"
|
style = "modern" # "modern" or "classic"
|
||||||
|
|||||||
@@ -43,6 +43,7 @@ type CoreConfig struct {
|
|||||||
// 0 = shell history, 1 = atuin only, 2 = atuin + shell
|
// 0 = shell history, 1 = atuin only, 2 = atuin + shell
|
||||||
Atuin int `toml:"atuin-history"`
|
Atuin int `toml:"atuin-history"`
|
||||||
AtuinDBPath string `toml:"atuin-db-path"`
|
AtuinDBPath string `toml:"atuin-db-path"`
|
||||||
|
CobraProbeEnabled bool `toml:"cobra-probe-enabled"`
|
||||||
}
|
}
|
||||||
|
|
||||||
type UIConfig struct {
|
type UIConfig struct {
|
||||||
|
|||||||
@@ -27,6 +27,9 @@ func TestDefaultConfigAndState(t *testing.T) {
|
|||||||
if cfg.AI.Providers != nil {
|
if cfg.AI.Providers != nil {
|
||||||
t.Errorf("expected default providers map to be nil, got %v", cfg.AI.Providers)
|
t.Errorf("expected default providers map to be nil, got %v", cfg.AI.Providers)
|
||||||
}
|
}
|
||||||
|
if !cfg.Core.CobraProbeEnabled {
|
||||||
|
t.Errorf("expected cobra probing to be enabled by default")
|
||||||
|
}
|
||||||
|
|
||||||
// test manual provider registration
|
// test manual provider registration
|
||||||
cfg.AI.Provider = "custom"
|
cfg.AI.Provider = "custom"
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ func DefaultConfig() *Config {
|
|||||||
Debug: false,
|
Debug: false,
|
||||||
ExpandAlias: true,
|
ExpandAlias: true,
|
||||||
AutoExecute: false,
|
AutoExecute: false,
|
||||||
|
CobraProbeEnabled: true,
|
||||||
},
|
},
|
||||||
UI: UIConfig{
|
UI: UIConfig{
|
||||||
Style: "modern",
|
Style: "modern",
|
||||||
|
|||||||
@@ -65,6 +65,9 @@ atuin-history = 0
|
|||||||
# custom atuin database path (leave empty for default)
|
# custom atuin database path (leave empty for default)
|
||||||
atuin-db-path = ""
|
atuin-db-path = ""
|
||||||
|
|
||||||
|
# probe unknown binaries with ` + "`__complete`" + ` for Cobra-based CLI suggestions.
|
||||||
|
cobra-probe-enabled = true
|
||||||
|
|
||||||
[ui]
|
[ui]
|
||||||
# visual style: "modern" (icons, category pills, shortcut footer) or "classic" (minimalist, centered number, no icons)
|
# visual style: "modern" (icons, category pills, shortcut footer) or "classic" (minimalist, centered number, no icons)
|
||||||
style = "modern"
|
style = "modern"
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package spec
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"debug/buildinfo"
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
"strconv"
|
"strconv"
|
||||||
@@ -9,8 +10,12 @@ import (
|
|||||||
"sync"
|
"sync"
|
||||||
"syscall"
|
"syscall"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/versenilvis/iris/internal/config"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
const cobraModulePath = "github.com/spf13/cobra"
|
||||||
|
|
||||||
type cobraCacheEntry struct {
|
type cobraCacheEntry struct {
|
||||||
suggestions []Suggestion
|
suggestions []Suggestion
|
||||||
}
|
}
|
||||||
@@ -91,6 +96,25 @@ func buildCobraCacheKey(binKey string, args []string, partial string) string {
|
|||||||
return sb.String()
|
return sb.String()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// isLikelyCobraBinary reports whether binName is a Go binary linking Cobra.
|
||||||
|
// only does static analysis so can produce false negatives and positives.
|
||||||
|
func isLikelyCobraBinary(binName string) bool {
|
||||||
|
path, err := exec.LookPath(binName)
|
||||||
|
if err != nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
info, err := buildinfo.ReadFile(path)
|
||||||
|
if err != nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
for _, dep := range info.Deps {
|
||||||
|
if dep.Path == cobraModulePath {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
// newProbeCmd builds and isolates the `__complete` probe command.
|
// newProbeCmd builds and isolates the `__complete` probe command.
|
||||||
// starts the child in its own session so it has no controlling terminal
|
// starts the child in its own session so it has no controlling terminal
|
||||||
// and therefore won't affect the user's tty in the case of programs that
|
// and therefore won't affect the user's tty in the case of programs that
|
||||||
@@ -108,6 +132,9 @@ func QueryCobraComplete(binName string, args []string, partial string) []Suggest
|
|||||||
if strings.ContainsAny(binName, `/\`) {
|
if strings.ContainsAny(binName, `/\`) {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
if !config.Get().Core.CobraProbeEnabled {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
binKey := cobraBinKey(binName)
|
binKey := cobraBinKey(binName)
|
||||||
argKey := buildCobraCacheKey(binKey, args, partial)
|
argKey := buildCobraCacheKey(binKey, args, partial)
|
||||||
@@ -119,6 +146,10 @@ func QueryCobraComplete(binName string, args []string, partial string) []Suggest
|
|||||||
}
|
}
|
||||||
cobraCacheMu.Unlock()
|
cobraCacheMu.Unlock()
|
||||||
|
|
||||||
|
if !isLikelyCobraBinary(binName) {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond)
|
ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond)
|
||||||
defer cancel()
|
defer cancel()
|
||||||
|
|
||||||
|
|||||||
@@ -8,6 +8,8 @@ import (
|
|||||||
"path/filepath"
|
"path/filepath"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/versenilvis/iris/internal/config"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestParseCobraOutput_ValidCobra(t *testing.T) {
|
func TestParseCobraOutput_ValidCobra(t *testing.T) {
|
||||||
@@ -166,6 +168,40 @@ func TestNewProbeCmd_NoControllingTerminal(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestQueryCobraComplete_ProbeDisabled(t *testing.T) {
|
||||||
|
t.Cleanup(ResetCobraCache)
|
||||||
|
original := config.Get()
|
||||||
|
t.Cleanup(func() { config.Init(original) })
|
||||||
|
|
||||||
|
cfg := config.DefaultConfig()
|
||||||
|
cfg.Core.CobraProbeEnabled = false
|
||||||
|
config.Init(cfg)
|
||||||
|
|
||||||
|
if result := QueryCobraComplete("non-go-binary", nil, ""); result != nil {
|
||||||
|
t.Errorf("expected nil when cobra probing is disabled, got %v", result)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestLooksLikeCobraBinary_NotGoBinary(t *testing.T) {
|
||||||
|
// 'ls' is a standard unix command that's not a go binary
|
||||||
|
if isLikelyCobraBinary("ls") {
|
||||||
|
t.Errorf("expected 'ls' to not look like a Cobra binary")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestLooksLikeCobraBinary_RealCobraBinary(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
binPath := filepath.Join(dir, "cobrafixture")
|
||||||
|
build := exec.CommandContext(context.Background(), "go", "build", "-o", binPath, "./testdata/cobrafixture")
|
||||||
|
if out, err := build.CombinedOutput(); err != nil {
|
||||||
|
t.Fatalf("could not build fixture binary: %v: %s", err, out)
|
||||||
|
}
|
||||||
|
|
||||||
|
if !isLikelyCobraBinary(binPath) {
|
||||||
|
t.Errorf("expected fixture binary linking Cobra to look like a Cobra binary")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestLookup_CobraGolangciLint(t *testing.T) {
|
func TestLookup_CobraGolangciLint(t *testing.T) {
|
||||||
results := Lookup("golangci-lint ")
|
results := Lookup("golangci-lint ")
|
||||||
if len(results) == 0 {
|
if len(results) == 0 {
|
||||||
|
|||||||
Vendored
+9
@@ -0,0 +1,9 @@
|
|||||||
|
package main
|
||||||
|
|
||||||
|
import "github.com/spf13/cobra"
|
||||||
|
|
||||||
|
// minimal Cobra CLI, built at test time as a fixture for TestLooksLikeCobraBinary_RealCobraBinary
|
||||||
|
func main() {
|
||||||
|
root := &cobra.Command{Use: "cobrafixture"}
|
||||||
|
_ = root.Execute()
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user