Add test command to verify version collisions do not occur (#2272)

* Add test command to verify version collisions do not occur
* add max_allowable_version to schema and lock flow
* add max_allowable_version to all entries in version.lock
* add test-version-lock command
* use min supported stack if > locked min stack
* share lock conversion code with rule and lock to fix M.m bug

(cherry picked from commit 2ee5a185c7)

detection_rules/devtools.py

@@ -637,6 +637,32 @@ def license_check(ctx, ignore_directory):
     ctx.exit(int(failed))
 
 
+@dev_group.command('test-version-lock')
+@click.argument('branches', nargs=-1, required=True)
+@click.option('--remote', '-r', default='origin', help='Override the remote from "origin"')
+def test_version_lock(branches: tuple, remote: str):
+    """Simulate the incremental step in the version locking to find version change violations."""
+    git = utils.make_git('-C', '.')
+    current_branch = git('rev-parse', '--abbrev-ref', 'HEAD')
+
+    try:
+        click.echo(f'iterating lock process for branches: {branches}')
+        for branch in branches:
+            click.echo(branch)
+            git('checkout', f'{remote}/{branch}')
+            subprocess.check_call(['python', '-m', 'detection_rules', 'dev', 'build-release', '-u'])
+
+    finally:
+        diff = git('--no-pager', 'diff', get_etc_path('version.lock.json'))
+        outfile = Path(get_path()).joinpath('lock-diff.txt')
+        outfile.write_text(diff)
+        click.echo(f'diff saved to {outfile}')
+
+        click.echo('reverting changes in version.lock')
+        git('checkout', '-f')
+        git('checkout', current_branch)
+
+
 @dev_group.command('package-stats')
 @click.option('--token', '-t', help='GitHub token to search API authenticated (may exceed threshold without auth)')
 @click.option('--threads', default=50, help='Number of threads to download rules from GitHub')

detection_rules/rule.py

@@ -342,7 +342,7 @@ class QueryValidator:
 
     @property
     def unique_fields(self) -> Any:
-        raise NotImplementedError
+        raise NotImplementedError()
 
     def validate(self, data: 'QueryRuleData', meta: RuleMeta) -> None:
         raise NotImplementedError()
@@ -585,16 +585,48 @@ class BaseRuleContents(ABC):
     @property
     def is_dirty(self) -> Optional[bool]:
         """Determine if the rule has changed since its version was locked."""
-        min_stack = self.metadata.get('min_stack_version') or str(get_min_supported_stack_version(drop_patch=True))
+        min_stack = self.get_supported_version()
         existing_sha256 = self.version_lock.get_locked_hash(self.id, min_stack)
 
         if existing_sha256 is not None:
             return existing_sha256 != self.sha256()
 
+    @property
+    def lock_entry(self) -> Optional[dict]:
+        lock_entry = self.version_lock.version_lock.data.get(self.id)
+        if lock_entry:
+            return lock_entry.to_dict()
+
+    @property
+    def has_forked(self) -> bool:
+        """Determine if the rule has forked at any point (has a previous entry)."""
+        lock_entry = self.lock_entry
+        if lock_entry:
+            return 'previous' in lock_entry
+        return False
+
+    @property
+    def is_in_forked_version(self) -> bool:
+        """Determine if the rule is in a forked version."""
+        if not self.has_forked:
+            return False
+        locked_min_stack = Version(self.lock_entry['min_stack_version'])
+        current_package_ver = Version(load_current_package_version())
+        return current_package_ver < locked_min_stack
+
+    def get_version_space(self) -> Optional[int]:
+        """Retrieve the number of version spaces available (None for unbound)."""
+        if self.is_in_forked_version:
+            current_entry = self.lock_entry['previous'][self.metadata.min_stack_version]
+            current_version = current_entry['version']
+            max_allowable_version = current_entry['max_allowable_version']
+
+            return max_allowable_version - current_version - 1
+
     @property
     def latest_version(self) -> Optional[int]:
         """Retrieve the latest known version of the rule."""
-        min_stack = self.metadata.get('min_stack_version') or str(get_min_supported_stack_version(drop_patch=True))
+        min_stack = self.get_supported_version()
         return self.version_lock.get_locked_version(self.id, min_stack)
 
     @property
@@ -606,6 +638,21 @@ class BaseRuleContents(ABC):
 
         return version + 1 if self.is_dirty else version
 
+    @classmethod
+    def convert_supported_version(cls, stack_version: Optional[str]) -> Version:
+        """Convert an optional stack version to the minimum for the lock in the form major.minor."""
+        min_version = get_min_supported_stack_version(drop_patch=True)
+        if stack_version is None:
+            return min_version
+        short_stack_version = Version(Version(stack_version)[:2])
+        return max(short_stack_version, min_version)
+
+    def get_supported_version(self) -> str:
+        """Get the lowest stack version for the rule that is currently supported in the form major.minor."""
+        rule_min_stack = self.metadata.get('min_stack_version')
+        min_stack = self.convert_supported_version(rule_min_stack)
+        return str(min_stack)
+
     def _post_dict_transform(self, obj: dict) -> dict:
         """Transform the converted API in place before sending to Kibana."""
 

detection_rules/semver.py

@@ -29,3 +29,8 @@ class Version(tuple):
                 recovered_str += "." + str(additional)
 
         return recovered_str
+
+
+def max_versions(*versions: str) -> str:
+    """Return the max versioned string."""
+    return str(max([Version(v) for v in versions]))

detection_rules/version_lock.py

@@ -12,7 +12,7 @@ import click
 
 from .mixins import LockDataclassMixin, MarshmallowDataclassMixin
 from .rule_loader import RuleCollection
-from .schemas import definitions, get_min_supported_stack_version
+from .schemas import definitions
 from .semver import Version
 from .utils import cached, get_etc_path
 
@@ -34,11 +34,19 @@ class BaseEntry:
     version: definitions.PositiveInteger
 
 
+@dataclass(frozen=True)
+class PreviousEntry(BaseEntry):
+
+    # this is Optional for resiliency in already tagged branches missing this field. This means we should strictly
+    # validate elsewhere
+    max_allowable_version: Optional[int]
+
+
 @dataclass(frozen=True)
 class VersionLockFileEntry(MarshmallowDataclassMixin, BaseEntry):
     """Schema for a rule entry in the version lock."""
     min_stack_version: Optional[definitions.SemVerMinorOnly]
-    previous: Optional[Dict[definitions.SemVerMinorOnly, BaseEntry]]
+    previous: Optional[Dict[definitions.SemVerMinorOnly, PreviousEntry]]
 
 
 @dataclass(frozen=True)
@@ -83,15 +91,6 @@ class DeprecatedRulesFile(LockDataclassMixin):
         return self.data[item]
 
 
-def _convert_lock_version(stack_version: Optional[str]) -> Version:
-    """Convert an optional stack version to the minimum for the lock."""
-    min_version = get_min_supported_stack_version(drop_patch=True)
-    if stack_version is None:
-        return min_version
-    short_stack_version = Version(Version(stack_version)[:2])
-    return max(short_stack_version, min_version)
-
-
 @cached
 def load_versions() -> dict:
     """Load and validate the default version.lock file."""
@@ -203,7 +202,7 @@ class VersionLock:
         for rule in rules:
             if rule.contents.metadata.maturity == "production" or rule.id in newly_deprecated:
                 # assume that older stacks are always locked first
-                min_stack = _convert_lock_version(rule.contents.metadata.min_stack_version)
+                min_stack = Version(rule.contents.get_supported_version())
 
                 lock_from_rule = rule.contents.lock_info(bump=not exclude_version_update)
                 lock_from_file: dict = lock_file_contents.setdefault(rule.id, {})
@@ -222,7 +221,8 @@ class VersionLock:
                 # 2) on the latest, after a breaking change has been locked
                 # 3) on the latest stack, locking in a breaking change
                 # 4) on an old stack, after a breaking change has been made
-                latest_locked_stack_version = _convert_lock_version(lock_from_file.get("min_stack_version"))
+                latest_locked_stack_version = rule.contents.convert_supported_version(
+                    lock_from_file.get("min_stack_version"))
 
                 if not lock_from_file or min_stack == latest_locked_stack_version:
                     route = 'A'
@@ -241,6 +241,7 @@ class VersionLock:
                     route = 'B'
                     # 3) on the latest stack, locking in a breaking change
                     previous_lock_info = {
+                        "max_allowable_version": lock_from_rule['version'] - 1,
                         "rule_name": lock_from_file["rule_name"],
                         "sha256": lock_from_file["sha256"],
                         "version": lock_from_file["version"],
@@ -272,11 +273,18 @@ class VersionLock:
                     #       We can still inspect the version lock manually after locks are made,
                     #       since it's a good summary of everything that happens
 
-                    # if version bump collides with future bump, fail
-                    # if space, change and log
+                    previous_entry = lock_from_file["previous"][str(min_stack)]
+                    max_allowable_version = previous_entry['max_allowable_version']
+
+                    # if version bump collides with future bump: fail
+                    # if space: change and log
                     info_from_rule = (lock_from_rule['sha256'], lock_from_rule['version'])
-                    info_from_file = (lock_from_file["previous"][str(min_stack)]['sha256'],
-                                      lock_from_file["previous"][str(min_stack)]['version'])
+                    info_from_file = (previous_entry['sha256'], previous_entry['version'])
+
+                    if lock_from_rule['version'] > max_allowable_version:
+                        raise ValueError(f'Forked rule: {rule.id} - {rule.name} has changes that will force it to '
+                                         f'exceed the max allowable version of {max_allowable_version}')
+
                     if info_from_rule != info_from_file:
                         lock_from_file["previous"][str(min_stack)] = lock_from_rule
                         new_version = lock_from_rule["version"]
@@ -286,14 +294,6 @@ class VersionLock:
                 else:
                     raise RuntimeError("Unreachable code")
 
-                if 'previous' in lock_from_file:
-                    current_rule_version = rule.contents.lock_info()['version']
-                    for min_stack_version, versioned_lock in lock_from_file['previous'].items():
-                        existing_lock_version = versioned_lock['version']
-                        if current_rule_version < existing_lock_version:
-                            raise ValueError(f'{rule.id} - previous {min_stack_version=} {existing_lock_version=} '
-                                             f'has a higher version than {current_rule_version=}')
-
         for rule in rules.deprecated:
             if rule.id in newly_deprecated:
                 current_deprecated_lock[rule.id] = {