fix: tighten commit skip directive matching

This commit is contained in:
harsh mahajan
2026-05-05 18:23:14 +05:30
parent a77558807c
commit 61cee5892a
2 changed files with 86 additions and 11 deletions
@@ -12,7 +12,7 @@ class CommitSkipPatterns extends Validator
/**
* Returns false (skip deployment) when the commit message contains any of the
* configured patterns (case-insensitive substring match).
* configured skip directives.
* Returns true (proceed) when no patterns are configured or none match.
*/
public function isValid($value): bool
@@ -21,11 +21,13 @@ class CommitSkipPatterns extends Validator
return false;
}
foreach ($this->patterns as $pattern) {
if (!is_string($pattern) || $pattern === '') {
continue;
}
if (stripos($value, $pattern) !== false) {
$patterns = $this->normalizePatterns($this->patterns);
if (empty($patterns)) {
return true;
}
foreach ($this->extractDirectives($value) as $directive) {
if (isset($patterns[$directive])) {
return false;
}
}
@@ -47,4 +49,64 @@ class CommitSkipPatterns extends Validator
{
return self::TYPE_STRING;
}
/**
* @param array<mixed> $patterns
* @return array<string, true>
*/
private function normalizePatterns(array $patterns): array
{
$normalized = [];
foreach ($patterns as $pattern) {
if (!\is_string($pattern)) {
continue;
}
$pattern = $this->normalizeDirective($pattern);
if ($pattern === '') {
continue;
}
$normalized[$pattern] = true;
}
return $normalized;
}
/**
* @return array<string>
*/
private function extractDirectives(string $message): array
{
$directives = [];
if (\preg_match_all('/\[[^\]\r\n]+\]/u', $message, $matches) > 0) {
foreach ($matches[0] as $match) {
$directives[] = $this->normalizeDirective($match);
}
}
foreach (\preg_split("/\r\n|\n|\r/", $message) ?: [] as $line) {
$line = \trim($line);
if ($line === '' || !\str_contains($line, ':')) {
continue;
}
$directives[] = $this->normalizeDirective($line);
}
return \array_values(\array_filter(\array_unique($directives)));
}
private function normalizeDirective(string $value): string
{
$value = \trim($value);
if ($value === '') {
return '';
}
$value = (string) \preg_replace('/\s+/u', ' ', $value);
return \mb_strtolower($value);
}
}
@@ -20,7 +20,7 @@ class CommitSkipPatternsTest extends TestCase
}
// -------------------------------------------------------------------------
// Single pattern — exact substring
// Single pattern — directive match
// -------------------------------------------------------------------------
public function testSinglePatternMatchSkips(): void
@@ -37,6 +37,7 @@ class CommitSkipPatternsTest extends TestCase
$this->assertTrue($validator->isValid('fix: real bug fix'));
$this->assertTrue($validator->isValid('feat: add new feature'));
$this->assertTrue($validator->isValid('skip deploy without brackets'));
$this->assertTrue($validator->isValid('prefix[skip deploy]suffix'));
}
// -------------------------------------------------------------------------
@@ -119,17 +120,15 @@ class CommitSkipPatternsTest extends TestCase
public function testBlankPatternsInArrayAreIgnored(): void
{
$validator = new CommitSkipPatterns(['', ' ', '[skip deploy]']);
// empty/whitespace-only patterns must not cause a false positive on empty messages
$this->assertTrue($validator->isValid('normal commit message'));
// but the real pattern still works
$this->assertFalse($validator->isValid('[skip deploy] docs'));
}
public function testPatternAsSubstringOfLongerWord(): void
public function testPatternMustBeStandaloneDirective(): void
{
// "skip" is a substring of "skippy" — should NOT accidentally skip
$validator = new CommitSkipPatterns(['[skip deploy]']);
$this->assertTrue($validator->isValid('skippy the kangaroo'));
$this->assertTrue($validator->isValid('prefix[skip deploy]suffix'));
}
public function testMultilineCommitMessage(): void
@@ -138,4 +137,18 @@ class CommitSkipPatternsTest extends TestCase
$msg = "feat: add new stuff\n\nMore detail here.\n\n[skip deploy]";
$this->assertFalse($validator->isValid($msg));
}
public function testWhitespaceInsideDirectiveIsNormalized(): void
{
$validator = new CommitSkipPatterns([' [skip deploy] ']);
$this->assertFalse($validator->isValid('[skip deploy] docs only'));
$this->assertFalse($validator->isValid('[SKIP DEPLOY] docs only'));
}
public function testTrailerDirectiveCanSkip(): void
{
$validator = new CommitSkipPatterns(['skip-checks: true']);
$msg = "feat: add new stuff\n\nMore detail here.\n\nskip-checks:true";
$this->assertFalse($validator->isValid($msg));
}
}