Skip to content

Conversation

@shashjar
Copy link
Member

@shashjar shashjar commented Nov 6, 2025

Adds a test to codify the matching behavior for CODEOWNERS rules when leading slashes are involved between the rule pattern & stack trace path.

https://sentry.slack.com/archives/C04KZQBNQ2U/p1762380421731289

@codecov
Copy link

codecov bot commented Nov 6, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
29533 1 29532 237
View the top 1 failed test(s) by shortest run time
tests.sentry.issues.ownership.test_grammar::test_codeowners_leading_slash_matching[libs/mobile/screens/home/**-path_details0-True]
Stack Traces | 0.051s run time
#x1B[1m#x1B[.../issues/ownership/test_grammar.py#x1B[0m:1200: in test_codeowners_leading_slash_matching
    _assert_matcher(Matcher("codeowners", pattern), path_details, expected)
#x1B[1m#x1B[.../issues/ownership/test_grammar.py#x1B[0m:538: in _assert_matcher
    assert matcher.test(frames, Matcher.munge_if_needed(frames)) == expected
#x1B[1m#x1B[31mE   AssertionError: assert False == True#x1B[0m
#x1B[1m#x1B[31mE    +  where False = <bound method Matcher.test of Matcher(type='codeowners', pattern='.../mobile/screens/home/**')>({'stacktrace': {'frames': [{'filename': '/.../mobile/screens/home/src/views/home/components/tiles/invest-tile.component.tsx'}, {'abs_path': '/.../mobile/screens/home/src/views/home/components/tiles/invest-tile.component.tsx'}]}}, ([{'filename': '/.../mobile/screens/home/src/views/home/components/tiles/invest-tile.component.tsx'}, {'abs_path': '/.../mobile/screens/home/src/views/home/components/tiles/invest-tile.component.tsx'}], ['filename', 'abs_path']))#x1B[0m
#x1B[1m#x1B[31mE    +    where <bound method Matcher.test of Matcher(type='codeowners', pattern='.../mobile/screens/home/**')> = Matcher(type='codeowners', pattern='.../mobile/screens/home/**').test#x1B[0m
#x1B[1m#x1B[31mE    +    and   ([{'filename': '/.../mobile/screens/home/src/views/home/components/tiles/invest-tile.component.tsx'}, {'abs_path': '/.../mobile/screens/home/src/views/home/components/tiles/invest-tile.component.tsx'}], ['filename', 'abs_path']) = <function Matcher.munge_if_needed at 0x7f363c8c4860>({'stacktrace': {'frames': [{'filename': '/.../mobile/screens/home/src/views/home/components/tiles/invest-tile.component.tsx'}, {'abs_path': '/.../mobile/screens/home/src/views/home/components/tiles/invest-tile.component.tsx'}]}})#x1B[0m
#x1B[1m#x1B[31mE    +      where <function Matcher.munge_if_needed at 0x7f363c8c4860> = Matcher.munge_if_needed#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@shashjar shashjar requested review from a team and cvxluo and removed request for a team November 11, 2025 17:49
@shashjar shashjar marked this pull request as ready for review November 11, 2025 17:50
@shashjar shashjar requested review from a team as code owners November 11, 2025 17:50
{"abs_path": "libs/web/views/index/home.tsx"},
],
True,
),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Root Anchoring Logic Inconsistency

The test expects an anchored pattern /libs/web/views/index/** (with leading slash) to match a path without a leading slash libs/web/views/index/home.tsx. This contradicts standard CODEOWNERS semantics where a leading slash anchors the pattern to the repository root, meaning it should only match paths that also start from the root. This creates an asymmetry with the first test case where a pattern without a leading slash doesn't match a path with one, suggesting the expected value should be False instead of True.

Fix in Cursor Fix in Web

@semgrep-code-getsentry
Copy link

Semgrep found 1 ssc-aecabbe1-e60d-9dc0-a5bd-95001ace2360 finding:

Risk: Affected versions of Django are vulnerable to Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection'). SQL injection in Django's ORM column aliases: when using QuerySet.annotate(), QuerySet.alias(), QuerySet.aggregate(), or QuerySet.extra() with dictionary expansion (**kwargs), the dictionary keys are used unescaped as SQL column aliases. On MySQL and MariaDB backends, an attacker who can influence those keys (for example, by passing a crafted dict of annotations) can inject arbitrary SQL into the generated query.

Manual Review Advice: A vulnerability from this advisory is reachable if you are using Django with MySQL or MariaDB

Fix: Upgrade this library to at least version 5.2.7 at sentry/uv.lock:305.

Reference(s): GHSA-hpr9-3m2g-3j9p, CVE-2025-59681

@shashjar shashjar merged commit 753774b into master Nov 11, 2025
62 checks passed
@shashjar shashjar deleted the repro-test-codeowners-pattern-path-leading-slashes branch November 11, 2025 18:14
Jesse-Box pushed a commit that referenced this pull request Nov 12, 2025
Adds a test to codify the matching behavior for CODEOWNERS rules when
leading slashes are involved between the rule pattern & stack trace
path.

https://sentry.slack.com/archives/C04KZQBNQ2U/p1762380421731289
andrewshie-sentry pushed a commit that referenced this pull request Nov 13, 2025
Adds a test to codify the matching behavior for CODEOWNERS rules when
leading slashes are involved between the rule pattern & stack trace
path.

https://sentry.slack.com/archives/C04KZQBNQ2U/p1762380421731289
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants