fix(validation): accept prefix when it's a prefix of allowed entry (GH#1135)
When ExtractIssuePrefix returns "hq" from an ID like "hq-cv-test" (because "test" is word-like), but "hq-cv" is in allowedPrefixes, we should accept "hq" since it's clearly intended to be part of "hq-cv". This handles cases where the prefix extraction algorithm yields a shorter prefix than the user intended, but the full intended prefix is in the allowed list. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
committed by
Steve Yegge
parent
356ab92b78
commit
c8ffcce621
@@ -87,19 +87,31 @@ func ValidatePrefix(requestedPrefix, dbPrefix string, force bool) error {
|
|||||||
// - dbPrefix is empty
|
// - dbPrefix is empty
|
||||||
// - requestedPrefix matches dbPrefix
|
// - requestedPrefix matches dbPrefix
|
||||||
// - requestedPrefix is in the comma-separated allowedPrefixes list
|
// - requestedPrefix is in the comma-separated allowedPrefixes list
|
||||||
|
// - requestedPrefix is a prefix of any entry in allowedPrefixes (GH#1135)
|
||||||
|
//
|
||||||
|
// The prefix-of-allowed check handles cases where ExtractIssuePrefix returns
|
||||||
|
// a shorter prefix than intended. For example, "hq-cv-test" extracts as "hq"
|
||||||
|
// (because "test" is word-like), but if "hq-cv" is in allowedPrefixes, we
|
||||||
|
// should accept "hq" since it's clearly intended to be part of "hq-cv".
|
||||||
// Returns an error if none of these conditions are met.
|
// Returns an error if none of these conditions are met.
|
||||||
func ValidatePrefixWithAllowed(requestedPrefix, dbPrefix, allowedPrefixes string, force bool) error {
|
func ValidatePrefixWithAllowed(requestedPrefix, dbPrefix, allowedPrefixes string, force bool) error {
|
||||||
if force || dbPrefix == "" || dbPrefix == requestedPrefix {
|
if force || dbPrefix == "" || dbPrefix == requestedPrefix {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if requestedPrefix is in the allowed list
|
// Check if requestedPrefix is in the allowed list or is a prefix of an allowed entry
|
||||||
if allowedPrefixes != "" {
|
if allowedPrefixes != "" {
|
||||||
for _, allowed := range strings.Split(allowedPrefixes, ",") {
|
for _, allowed := range strings.Split(allowedPrefixes, ",") {
|
||||||
allowed = strings.TrimSpace(allowed)
|
allowed = strings.TrimSpace(allowed)
|
||||||
if allowed == requestedPrefix {
|
if allowed == requestedPrefix {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
// GH#1135: Also accept if requestedPrefix is a prefix of an allowed entry.
|
||||||
|
// This handles IDs like "hq-cv-test" where extraction yields "hq" but
|
||||||
|
// the user configured "hq-cv" in allowed_prefixes.
|
||||||
|
if strings.HasPrefix(allowed, requestedPrefix+"-") {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -314,6 +314,14 @@ func TestValidatePrefixWithAllowed(t *testing.T) {
|
|||||||
{"allowed with spaces", "gt", "hq", "gt, hmc, foo", false, false},
|
{"allowed with spaces", "gt", "hq", "gt, hmc, foo", false, false},
|
||||||
{"empty allowed list", "gt", "hq", "", false, true},
|
{"empty allowed list", "gt", "hq", "", false, true},
|
||||||
{"single allowed prefix", "gt", "hq", "gt", false, false},
|
{"single allowed prefix", "gt", "hq", "gt", false, false},
|
||||||
|
|
||||||
|
// GH#1135: prefix-of-allowed cases
|
||||||
|
// When ExtractIssuePrefix returns "hq" from "hq-cv-test", but "hq-cv" is allowed
|
||||||
|
{"GH#1135 prefix-of-allowed hq->hq-cv", "hq", "djdefi-ops", "djdefi-ops,hq-cv", false, false},
|
||||||
|
{"GH#1135 prefix-of-allowed with multiple", "hq", "djdefi-ops", "hq-cv,hq-other,foo", false, false},
|
||||||
|
{"GH#1135 exact match still works", "hq-cv", "djdefi-ops", "hq-cv", false, false},
|
||||||
|
{"GH#1135 no false positive for unrelated prefix", "bar", "djdefi-ops", "hq-cv", false, true},
|
||||||
|
{"GH#1135 no false positive for partial overlap", "hq", "djdefi-ops", "hqx-cv", false, true},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
|
|||||||
Reference in New Issue
Block a user