fix(security): validate beads prefix to prevent command injection (gt-l1xsa)
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 <noreply@anthropic.com>
This commit is contained in:
committed by
Steve Yegge
parent
b50d2a6fdb
commit
18578b3030
@@ -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
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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())
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user