From 18578b30309e78749aedb97150114f4a365926e2 Mon Sep 17 00:00:00 2001 From: gastown/crew/joe Date: Mon, 5 Jan 2026 00:02:36 -0800 Subject: [PATCH] fix(security): validate beads prefix to prevent command injection (gt-l1xsa) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add isValidBeadsPrefix() to validate prefix format before passing to exec.Command. Prefixes from config files (detectBeadsPrefixFromConfig) are now validated to contain only alphanumeric and hyphen characters, start with a letter, and be max 20 chars. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/rig/manager.go | 26 ++++++++++++-- internal/rig/manager_test.go | 67 ++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 3 deletions(-) diff --git a/internal/rig/manager.go b/internal/rig/manager.go index 2bf4f09b..5c7a0ab1 100644 --- a/internal/rig/manager.go +++ b/internal/rig/manager.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "strings" "time" @@ -351,7 +352,7 @@ func (m *Manager) AddRig(opts AddRigOptions) (*Rig, error) { // bd init --prefix will create the database and auto-import from issues.jsonl. sourceBeadsDB := filepath.Join(mayorRigPath, ".beads", "beads.db") if _, err := os.Stat(sourceBeadsDB); os.IsNotExist(err) { - cmd := exec.Command("bd", "init", "--prefix", sourcePrefix) //nolint:gosec // G204: bd is a trusted internal tool + cmd := exec.Command("bd", "init", "--prefix", sourcePrefix) // sourcePrefix validated by isValidBeadsPrefix cmd.Dir = mayorRigPath if output, err := cmd.CombinedOutput(); err != nil { fmt.Printf(" Warning: Could not init bd database: %v (%s)\n", err, strings.TrimSpace(string(output))) @@ -500,6 +501,11 @@ func LoadRigConfig(rigPath string) (*RigConfig, error) { // Use `bd doctor --fix` in the project to configure sync-branch if needed. // TODO(bd-yaml): beads config should migrate to JSON (see beads issue) func (m *Manager) initBeads(rigPath, prefix string) error { + // Validate prefix format to prevent command injection from config files + if !isValidBeadsPrefix(prefix) { + return fmt.Errorf("invalid beads prefix %q: must be alphanumeric with optional hyphens, start with letter, max 20 chars", prefix) + } + beadsDir := filepath.Join(rigPath, ".beads") if err := os.MkdirAll(beadsDir, 0755); err != nil { return err @@ -677,6 +683,18 @@ func deriveBeadsPrefix(name string) string { // Returns empty string if the file doesn't exist or doesn't contain a prefix. // Falls back to detecting prefix from existing issues in issues.jsonl. // +// beadsPrefixRegexp validates beads prefix format: alphanumeric, may contain hyphens, +// must start with letter, max 20 chars. Prevents shell injection via config files. +var beadsPrefixRegexp = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9-]{0,19}$`) + +// isValidBeadsPrefix checks if a prefix is safe for use in shell commands. +// Prefixes must be alphanumeric (with optional hyphens), start with a letter, +// and be at most 20 characters. This prevents command injection from +// malicious config files. +func isValidBeadsPrefix(prefix string) bool { + return beadsPrefixRegexp.MatchString(prefix) +} + // When adding a rig from a source repo that has .beads/ tracked in git (like a project // that already uses beads for issue tracking), we need to use that project's existing // prefix instead of generating a new one. Otherwise, the rig would have a mismatched @@ -702,7 +720,7 @@ func detectBeadsPrefixFromConfig(configPath string) string { value := strings.TrimSpace(strings.TrimPrefix(line, key)) // Remove quotes if present value = strings.Trim(value, `"'`) - if value != "" { + if value != "" && isValidBeadsPrefix(value) { return value } } @@ -729,7 +747,9 @@ func detectBeadsPrefixFromConfig(configPath string) string { if dashIdx := strings.LastIndex(issueID, "-"); dashIdx > 0 { prefix := issueID[:dashIdx] // Handle prefixes like "gt" (from "gt-abc") - return without trailing hyphen - return prefix + if isValidBeadsPrefix(prefix) { + return prefix + } } } } diff --git a/internal/rig/manager_test.go b/internal/rig/manager_test.go index c380441f..573196a6 100644 --- a/internal/rig/manager_test.go +++ b/internal/rig/manager_test.go @@ -410,3 +410,70 @@ esac } } } + +func TestIsValidBeadsPrefix(t *testing.T) { + tests := []struct { + prefix string + want bool + }{ + // Valid prefixes + {"gt", true}, + {"bd", true}, + {"hq", true}, + {"gastown", true}, + {"myProject", true}, + {"my-project", true}, + {"a", true}, + {"A", true}, + {"test123", true}, + {"a1b2c3", true}, + {"a-b-c", true}, + + // Invalid prefixes + {"", false}, // empty + {"1abc", false}, // starts with number + {"-abc", false}, // starts with hyphen + {"abc def", false}, // contains space + {"abc;ls", false}, // shell injection attempt + {"$(whoami)", false}, // command substitution + {"`id`", false}, // backtick command + {"abc|cat", false}, // pipe + {"../etc/passwd", false}, // path traversal + {"aaaaaaaaaaaaaaaaaaaaa", false}, // too long (21 chars, >20 limit) + {"valid-but-with-$var", false}, // variable reference + } + + for _, tt := range tests { + t.Run(tt.prefix, func(t *testing.T) { + got := isValidBeadsPrefix(tt.prefix) + if got != tt.want { + t.Errorf("isValidBeadsPrefix(%q) = %v, want %v", tt.prefix, got, tt.want) + } + }) + } +} + +func TestInitBeadsRejectsInvalidPrefix(t *testing.T) { + rigPath := t.TempDir() + manager := &Manager{} + + tests := []string{ + "", + "$(whoami)", + "abc;rm -rf /", + "../etc", + "123", + } + + for _, prefix := range tests { + t.Run(prefix, func(t *testing.T) { + err := manager.initBeads(rigPath, prefix) + if err == nil { + t.Errorf("initBeads(%q) should have failed", prefix) + } + if !strings.Contains(err.Error(), "invalid beads prefix") { + t.Errorf("initBeads(%q) error = %q, want error containing 'invalid beads prefix'", prefix, err.Error()) + } + }) + } +}