fix(create): validate explicit IDs against allowed_prefixes using starts-with matching (#1137)
When creating issues with explicit IDs like `bd create --id hq-cv-test`,
the prefix validation was failing even when `hq-cv` was in `allowed_prefixes`.
Root cause: `ExtractIssuePrefix("hq-cv-test")` returns `"hq"` (not `"hq-cv"`)
because "test" looks like an English word, causing the algorithm to fall back
to the first hyphen. The validation then checked if `"hq"` was in the allowed
list containing `"hq-cv"` - which failed.
The fix adds `ValidateIDPrefixAllowed()` which validates the full ID using
"starts with" matching (the same approach the importer uses successfully).
This correctly handles multi-hyphen prefixes like `hq-cv-` regardless of
what the suffix looks like.
Fixes #1135
Co-authored-by: Steven Syrek <steven.syrek@deepl.com>
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -400,20 +400,15 @@ var createCmd = &cobra.Command{
|
|||||||
|
|
||||||
// Validate explicit ID format if provided
|
// Validate explicit ID format if provided
|
||||||
if explicitID != "" {
|
if explicitID != "" {
|
||||||
var requestedPrefix string
|
// For agent types, use agent-aware validation.
|
||||||
var err error
|
|
||||||
|
|
||||||
// For agent types, use agent-aware prefix extraction.
|
|
||||||
// This fixes the bug where 3-char polecat names like "nux" in
|
// This fixes the bug where 3-char polecat names like "nux" in
|
||||||
// "nx-nexus-polecat-nux" were incorrectly treated as hash suffixes,
|
// "nx-nexus-polecat-nux" were incorrectly treated as hash suffixes.
|
||||||
// causing prefix to be extracted as "nx-nexus-polecat" instead of "nx".
|
|
||||||
if issueType == "agent" {
|
if issueType == "agent" {
|
||||||
if err := validation.ValidateAgentID(explicitID); err != nil {
|
if err := validation.ValidateAgentID(explicitID); err != nil {
|
||||||
FatalError("invalid agent ID: %v", err)
|
FatalError("invalid agent ID: %v", err)
|
||||||
}
|
}
|
||||||
requestedPrefix = validation.ExtractAgentPrefix(explicitID)
|
|
||||||
} else {
|
} else {
|
||||||
requestedPrefix, err = validation.ValidateIDFormat(explicitID)
|
_, err := validation.ValidateIDFormat(explicitID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
FatalError("%v", err)
|
FatalError("%v", err)
|
||||||
}
|
}
|
||||||
@@ -442,7 +437,10 @@ var createCmd = &cobra.Command{
|
|||||||
allowedPrefixes, _ = store.GetConfig(ctx, "allowed_prefixes")
|
allowedPrefixes, _ = store.GetConfig(ctx, "allowed_prefixes")
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := validation.ValidatePrefixWithAllowed(requestedPrefix, dbPrefix, allowedPrefixes, forceCreate); err != nil {
|
// Use ValidateIDPrefixAllowed which handles multi-hyphen prefixes correctly (GH#1135)
|
||||||
|
// This checks if the ID starts with an allowed prefix, rather than extracting
|
||||||
|
// the prefix first (which can fail for IDs like "hq-cv-test" where "test" looks like a word)
|
||||||
|
if err := validation.ValidateIDPrefixAllowed(explicitID, dbPrefix, allowedPrefixes, forceCreate); err != nil {
|
||||||
FatalError("%v", err)
|
FatalError("%v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -123,6 +123,49 @@ func ValidatePrefixWithAllowed(requestedPrefix, dbPrefix, allowedPrefixes string
|
|||||||
return fmt.Errorf("prefix mismatch: database uses '%s' but you specified '%s' (use --force to override)", dbPrefix, requestedPrefix)
|
return fmt.Errorf("prefix mismatch: database uses '%s' but you specified '%s' (use --force to override)", dbPrefix, requestedPrefix)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ValidateIDPrefixAllowed checks that an issue ID's prefix is allowed.
|
||||||
|
// Unlike ValidatePrefixWithAllowed which takes an extracted prefix, this function
|
||||||
|
// takes the full ID and checks if it starts with any allowed prefix.
|
||||||
|
// This correctly handles multi-hyphen prefixes like "hq-cv-" where the suffix
|
||||||
|
// might look like an English word (e.g., "hq-cv-test").
|
||||||
|
// (GH#1135)
|
||||||
|
//
|
||||||
|
// It matches if:
|
||||||
|
// - force is true
|
||||||
|
// - dbPrefix is empty
|
||||||
|
// - id starts with dbPrefix + "-"
|
||||||
|
// - id starts with any prefix in allowedPrefixes + "-"
|
||||||
|
// Returns an error if none of these conditions are met.
|
||||||
|
func ValidateIDPrefixAllowed(id, dbPrefix, allowedPrefixes string, force bool) error {
|
||||||
|
if force || dbPrefix == "" {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check if ID starts with the database prefix
|
||||||
|
if strings.HasPrefix(id, dbPrefix+"-") {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check if ID starts with any allowed prefix
|
||||||
|
if allowedPrefixes != "" {
|
||||||
|
for _, allowed := range strings.Split(allowedPrefixes, ",") {
|
||||||
|
allowed = strings.TrimSpace(allowed)
|
||||||
|
// Normalize: remove trailing - if present (we add it for matching)
|
||||||
|
allowed = strings.TrimSuffix(allowed, "-")
|
||||||
|
if allowed != "" && strings.HasPrefix(id, allowed+"-") {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Build helpful error message
|
||||||
|
if allowedPrefixes != "" {
|
||||||
|
return fmt.Errorf("prefix mismatch: database uses '%s-' (allowed: %s) but ID '%s' doesn't match any allowed prefix (use --force to override)",
|
||||||
|
dbPrefix, allowedPrefixes, id)
|
||||||
|
}
|
||||||
|
return fmt.Errorf("prefix mismatch: database uses '%s-' but ID '%s' doesn't match (use --force to override)", dbPrefix, id)
|
||||||
|
}
|
||||||
|
|
||||||
// ValidAgentRoles are the known agent role types for ID pattern validation
|
// ValidAgentRoles are the known agent role types for ID pattern validation
|
||||||
var ValidAgentRoles = []string{
|
var ValidAgentRoles = []string{
|
||||||
"mayor", // Town-level: gt-mayor
|
"mayor", // Town-level: gt-mayor
|
||||||
|
|||||||
@@ -334,6 +334,56 @@ func TestValidatePrefixWithAllowed(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestValidateIDPrefixAllowed tests the new function that validates IDs using
|
||||||
|
// "starts with" matching to handle multi-hyphen prefixes correctly (GH#1135).
|
||||||
|
func TestValidateIDPrefixAllowed(t *testing.T) {
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
id string
|
||||||
|
dbPrefix string
|
||||||
|
allowedPrefixes string
|
||||||
|
force bool
|
||||||
|
wantError bool
|
||||||
|
}{
|
||||||
|
// Basic cases
|
||||||
|
{"matching prefix", "bd-abc123", "bd", "", false, false},
|
||||||
|
{"empty db prefix", "bd-abc123", "", "", false, false},
|
||||||
|
{"mismatched with force", "foo-abc123", "bd", "", true, false},
|
||||||
|
{"mismatched without force", "foo-abc123", "bd", "", false, true},
|
||||||
|
|
||||||
|
// Multi-hyphen prefix cases (GH#1135 - the main bug)
|
||||||
|
{"hq-cv prefix with word suffix", "hq-cv-test", "djdefi-ops", "hq,hq-cv", false, false},
|
||||||
|
{"hq-cv prefix with hash suffix", "hq-cv-abc123", "djdefi-ops", "hq,hq-cv", false, false},
|
||||||
|
{"djdefi-ops with word suffix", "djdefi-ops-test", "djdefi-ops", "", false, false},
|
||||||
|
|
||||||
|
// Allowed prefixes list
|
||||||
|
{"allowed prefix gt", "gt-abc123", "hq", "gt,hmc", false, false},
|
||||||
|
{"allowed prefix hmc", "hmc-abc123", "hq", "gt,hmc", false, false},
|
||||||
|
{"primary prefix still works", "hq-abc123", "hq", "gt,hmc", false, false},
|
||||||
|
{"prefix not in allowed list", "foo-abc123", "hq", "gt,hmc", false, true},
|
||||||
|
|
||||||
|
// Edge cases
|
||||||
|
{"allowed with spaces", "gt-abc123", "hq", "gt, hmc, foo", false, false},
|
||||||
|
{"allowed with trailing dash", "gt-abc123", "hq", "gt-, hmc-", false, false},
|
||||||
|
{"empty allowed list", "gt-abc123", "hq", "", false, true},
|
||||||
|
{"single allowed prefix", "gt-abc123", "hq", "gt", false, false},
|
||||||
|
|
||||||
|
// Multi-hyphen allowed prefixes
|
||||||
|
{"multi-hyphen in allowed list", "my-cool-prefix-abc123", "hq", "my-cool-prefix,other", false, false},
|
||||||
|
{"partial match should fail", "hq-cv-extra-test", "hq", "hq-cv-extra", false, false},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
err := ValidateIDPrefixAllowed(tt.id, tt.dbPrefix, tt.allowedPrefixes, tt.force)
|
||||||
|
if (err != nil) != tt.wantError {
|
||||||
|
t.Errorf("ValidateIDPrefixAllowed(%q, %q, %q) error = %v, wantError %v",
|
||||||
|
tt.id, tt.dbPrefix, tt.allowedPrefixes, err, tt.wantError)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestValidateAgentID(t *testing.T) {
|
func TestValidateAgentID(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
|
|||||||
Reference in New Issue
Block a user