From a559980601043eef123a6f44a99e3669518305b7 Mon Sep 17 00:00:00 2001 From: Cheruku Sri Charan Reddy <185475071+sricharanreddycheruku@users.noreply.github.com> Date: Wed, 7 Oct 2026 05:00:14 +0000 Subject: [PATCH] Allow application-defined auditing of normalized gitignore segments --- CHANGES.rst | 2 + README.rst | 31 ++++++ doc/source/api.rst | 2 + pathspec/patterns/gitignore/base.py | 18 ++++ pathspec/patterns/gitignore/basic.py | 4 + pathspec/patterns/gitignore/spec.py | 4 + tests/check_usage.py | 22 ++++ tests/test_08_gitignore_audit.py | 151 +++++++++++++++++++++++++++ 8 files changed, 234 insertions(+) create mode 100644 tests/test_08_gitignore_audit.py diff --git a/CHANGES.rst b/CHANGES.rst index b3e7efc..9c2af88 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -15,6 +15,7 @@ API changes: New features: +- `Issue #131`_: Add an overridable ``_audit_segments()`` hook to gitignore patterns for application-specific complexity policies. - `Issue #126`_: `.iter_tree_files()` / `.iter_tree_entries()` methods now have a *subdir* parameter to allow traversing only part of the tree. - `GitIgnoreBasicPattern` and `GitIgnoreSpecPattern` now accept an `errors` argument to `__init__()` and `pattern_to_regex()` to control how invalid patterns are handled. @@ -43,6 +44,7 @@ Bug fixes: .. _`Pull #128`: https://github.com/cpburnz/python-pathspec/pull/128 .. _`Issue #129`: https://github.com/cpburnz/python-pathspec/issues/129 .. _`Pull #132`: https://github.com/cpburnz/python-pathspec/pull/132 +.. _`Issue #131`: https://github.com/cpburnz/python-pathspec/issues/131 .. _`Pull #133`: https://github.com/cpburnz/python-pathspec/pull/133 .. _`Issue #134`: https://github.com/cpburnz/python-pathspec/issues/134 .. _`Pull #135`: https://github.com/cpburnz/python-pathspec/pull/135 diff --git a/README.rst b/README.rst index 9824fb9..778b9b9 100644 --- a/README.rst +++ b/README.rst @@ -94,6 +94,37 @@ You do not specify the style of pattern for ``GitIgnoreSpec`` because it should always use ``GitIgnoreSpecPattern`` internally. +Auditing pattern complexity +--------------------------- + +Applications processing patterns from external sources can apply their own +complexity policy before regular expressions are compiled. Subclass +``GitIgnoreSpecPattern`` (or ``GitIgnoreBasicPattern`` for ``PathSpec``) and +override ``_audit_segments()``. Pass that subclass as the ``pattern_factory``:: + + >>> from pathspec.patterns.gitignore.spec import GitIgnoreSpecPattern + >>> class LimitedPattern(GitIgnoreSpecPattern): + ... @classmethod + ... def _audit_segments(cls, segments: tuple[str, ...]) -> None: + ... if segments.count('**') > 3: + ... raise ValueError('Too many recursive wildcards') + ... + >>> spec = GitIgnoreSpec.from_lines(['src/**/generated/**'], pattern_factory=LimitedPattern) + +The limit above is an application example; the standard pattern classes impose +no complexity limits. Auditing is specific to the supplied subclass and works +with every matching backend. It receives an immutable tuple of normalized +segments, with adjacent ``**`` segments collapsed and implicit recursive +wildcards included. Byte patterns are decoded first. The hook is also called +for patterns handled by a regular-expression override, such as ``*`` or ``**``. +Empty patterns, comments and precompiled regular expressions are not audited. + +Exceptions from the hook propagate unchanged, including when ``errors='null'`` +or ``errors='literal'`` is used for invalid pattern notation. Auditing can reject +patterns before compilation; a wildcard-count limit alone does not guarantee +that matching will take bounded time. + + Performance ----------- diff --git a/doc/source/api.rst b/doc/source/api.rst index be3d765..63c36e7 100644 --- a/doc/source/api.rst +++ b/doc/source/api.rst @@ -93,6 +93,7 @@ pathspec.patterns.gitignore.basic .. autoclass:: GitIgnoreBasicPattern :members: + :private-members: _audit_segments :inherited-members: :show-inheritance: @@ -104,6 +105,7 @@ pathspec.patterns.gitignore.spec .. autoclass:: GitIgnoreSpecPattern :members: + :private-members: _audit_segments :inherited-members: :show-inheritance: diff --git a/pathspec/patterns/gitignore/base.py b/pathspec/patterns/gitignore/base.py index 135a66c..3c7dd1b 100644 --- a/pathspec/patterns/gitignore/base.py +++ b/pathspec/patterns/gitignore/base.py @@ -145,6 +145,24 @@ def __init__( """ super().__init__(pattern, include, errors=errors) + @classmethod + def _audit_segments(cls, pattern_segs: tuple[str, ...], /) -> None: + """ + Audit normalized pattern segments before translating them to a regex. + + Subclasses may override this hook to enforce application-specific pattern + limits by raising an exception. The default implementation does nothing. + + *pattern_segs* (:class:`tuple` of :class:`str`) contains the normalized + segments, including implicit double-asterisks and with adjacent recursive + wildcards collapsed. Byte patterns are decoded before auditing. The tuple + cannot be changed by the hook. + + The hook also runs for patterns with a regex override, but not for null + patterns, comments or precompiled regular expressions. Exceptions from the + hook propagate unchanged, regardless of the notation *errors* policy. + """ + @staticmethod def escape(s: AnyStr) -> AnyStr: """ diff --git a/pathspec/patterns/gitignore/basic.py b/pathspec/patterns/gitignore/basic.py index f396a71..31fe333 100644 --- a/pathspec/patterns/gitignore/basic.py +++ b/pathspec/patterns/gitignore/basic.py @@ -245,6 +245,10 @@ def pattern_to_regex( f"Invalid git pattern: {original_pattern!r}" )) from e # GitIgnorePatternError + # Normalization modifies orig_segs in place, including regex overrides. + # Audit outside the notation-error handlers so rejections propagate. + cls._audit_segments(tuple(orig_segs)) + if override_regex is not None: # Use regex override. regex = override_regex diff --git a/pathspec/patterns/gitignore/spec.py b/pathspec/patterns/gitignore/spec.py index a443868..de4da7a 100644 --- a/pathspec/patterns/gitignore/spec.py +++ b/pathspec/patterns/gitignore/spec.py @@ -275,6 +275,10 @@ def pattern_to_regex( f"Invalid git pattern: {original_pattern!r}" )) from e # GitIgnorePatternError + # Normalization modifies orig_segs in place, including regex overrides. + # Audit outside the notation-error handlers so rejections propagate. + cls._audit_segments(tuple(orig_segs)) + if override_regex is not None: # Use regex override. regex = override_regex diff --git a/tests/check_usage.py b/tests/check_usage.py index a035c94..2ec1aa1 100644 --- a/tests/check_usage.py +++ b/tests/check_usage.py @@ -5,6 +5,8 @@ GitIgnoreSpec) from pathspec.patterns.gitignore.basic import ( GitIgnoreBasicPattern) +from pathspec.patterns.gitignore.spec import ( + GitIgnoreSpecPattern) def check_gi_1(): @@ -47,3 +49,23 @@ def pattern_factory(pattern: AnyStr) -> GitIgnoreBasicPattern: spec = PathSpec.from_lines(pattern_factory, ['**']) return spec + + +class LimitedBasicPattern(GitIgnoreBasicPattern): + @classmethod + def _audit_segments(cls, segments: tuple[str, ...]) -> None: + if segments.count('**') > 3: + raise ValueError('Too many recursive wildcards.') + + +class LimitedSpecPattern(GitIgnoreSpecPattern): + @classmethod + def _audit_segments(cls, segments: tuple[str, ...]) -> None: + if segments.count('**') > 3: + raise ValueError('Too many recursive wildcards.') + + +def check_audited_factories(): + path_spec = PathSpec.from_lines(LimitedBasicPattern, ['src/**']) + git_spec = GitIgnoreSpec.from_lines(['src/**'], pattern_factory=LimitedSpecPattern) + return path_spec, git_spec diff --git a/tests/test_08_gitignore_audit.py b/tests/test_08_gitignore_audit.py new file mode 100644 index 0000000..4082b5b --- /dev/null +++ b/tests/test_08_gitignore_audit.py @@ -0,0 +1,151 @@ +""" +Tests application-defined auditing of normalized gitignore patterns. +""" + +import re +from itertools import product +from unittest import TestCase +from unittest.mock import patch + +from pathspec import GitIgnoreSpec, PathSpec +from pathspec.patterns.gitignore.basic import GitIgnoreBasicPattern +from pathspec.patterns.gitignore.spec import GitIgnoreSpecPattern + +from .util import require_backend + + +class PatternAuditTest(TestCase): + """ + Tests that pattern factories can enforce application-specific limits. + """ + + def test_normalized_segments(self): + """ + Audit decoded segments after anchoring, directory and wildcard normalization. + """ + cases = [ + ('file', ('**', 'file')), + ('/file', ('file',)), + ('!foo/**/**/bar/', ('foo', '**', 'bar', '**')), + ('foo/\u00e9.txt', ('foo', '\u00e9.txt')), + ('**/**', ('**',)), + ('**/', ('**',)), + ('*', ('**', '*')), + ('*/', ('**', '*', '**')), + ('foo/bar \n', ('foo', 'bar')), + ] + for pattern_class, (pattern, expected), binary in product( + (GitIgnoreBasicPattern, GitIgnoreSpecPattern), cases, (False, True), + ): + with self.subTest(pattern_class=pattern_class, pattern=pattern, binary=binary): + seen = [] + + class AuditedPattern(pattern_class): + @classmethod + def _audit_segments(cls, segments): + seen.append(segments) + + text = pattern.encode('latin1') if binary else pattern + actual = AuditedPattern(text) + original = pattern_class(text) + self.assertEqual(seen, [expected]) + self.assertIsInstance(seen[0], tuple) + self.assertEqual(actual.include, original.include) + self.assertEqual(actual.regex, original.regex) + + def test_null_and_precompiled_patterns(self): + """ + Comments, empty patterns and precompiled expressions have no segments to audit. + """ + for pattern_class in (GitIgnoreBasicPattern, GitIgnoreSpecPattern): + seen = [] + + class AuditedPattern(pattern_class): + @classmethod + def _audit_segments(cls, segments): + seen.append(segments) + + for text in ('', '!', '# comment', b'', b'!', b'# comment', None): + with self.subTest(pattern_class=pattern_class, pattern=text): + self.assertIsNone(AuditedPattern(text).include) + self.assertEqual(seen, []) + + compiled = re.compile('foo') + self.assertIs(AuditedPattern(compiled, True).regex, compiled) + self.assertEqual(seen, []) + + def test_audit_exception_propagates(self): + """ + Audit rejection is not swallowed or replaced by notation error handling. + """ + for pattern_class, errors in product( + (GitIgnoreBasicPattern, GitIgnoreSpecPattern), + (None, 'literal', 'null', 'raise'), + ): + with self.subTest(pattern_class=pattern_class, errors=errors): + rejection = ValueError('Application pattern limit exceeded.') + + class AuditedPattern(pattern_class): + @classmethod + def _audit_segments(cls, segments): + raise rejection + + with patch('pathspec.pattern.re.compile') as compile_regex: + with self.assertRaises(ValueError) as caught: + AuditedPattern('a/**/b/**/c', errors=errors) + self.assertIs(caught.exception, rejection) + compile_regex.assert_not_called() + + def test_factories_and_backends(self): + """ + Both spec types honor an audited factory before selecting a matching backend. + """ + for spec_class, pattern_class in ( + (PathSpec, GitIgnoreBasicPattern), + (GitIgnoreSpec, GitIgnoreSpecPattern), + ): + class AuditedPattern(pattern_class): + @classmethod + def _audit_segments(cls, segments): + if segments.count('**') > 1: + raise ValueError('Too many recursive wildcards.') + + for backend in ('simple', 're2', 'hyperscan', 'best'): + with self.subTest(spec_class=spec_class, backend=backend): + require_backend(backend) + with self.assertRaisesRegex(ValueError, 'Too many recursive wildcards'): + spec_class.from_lines( + AuditedPattern, ['a/**/b/**/c'], backend=backend, + ) + spec = spec_class.from_lines( + AuditedPattern, ['foo/**/**/bar', '!foo/private/bar'], backend=backend, + ) + self.assertTrue(spec.match_file('foo/other/bar')) + self.assertFalse(spec.match_file('foo/private/bar')) + + def test_independent_policies(self): + """ + A subclass policy does not change standard patterns or another subclass. + """ + class RejectingPattern(GitIgnoreSpecPattern): + @classmethod + def _audit_segments(cls, segments): + raise ValueError('Rejected by this application.') + + class PermissivePattern(GitIgnoreSpecPattern): + @classmethod + def _audit_segments(cls, segments): + pass + + with self.assertRaises(ValueError): + RejectingPattern('foo/**/bar') + self.assertIsNotNone(PermissivePattern('foo/**/bar').match_file('foo/bar')) + self.assertIsNotNone(GitIgnoreSpecPattern('foo/**/bar').match_file('foo/bar')) + + def test_default_has_no_limits(self): + """ + Applications opt in; the standard factories impose no arbitrary limits. + """ + for pattern_class in (GitIgnoreBasicPattern, GitIgnoreSpecPattern): + pattern = pattern_class('a/' + '**/b/' * 10 + 'c') + self.assertIsNotNone(pattern.regex)