Skip to content

fix(policy): directory-style path patterns never match in conformance gating#52

Open
MoneyBund wants to merge 4 commits into
tempoxyz:mainfrom
MoneyBund:fix/policy-path-matching
Open

fix(policy): directory-style path patterns never match in conformance gating#52
MoneyBund wants to merge 4 commits into
tempoxyz:mainfrom
MoneyBund:fix/policy-path-matching

Conversation

@MoneyBund

Copy link
Copy Markdown

fix(policy): directory-style path patterns never match in conformance gating

Problem

path_matches() — duplicated across three CI policy scripts — has an unreachable
branch. The pattern is normalized with pattern.strip("/"), which removes the
trailing slash, so the directory-prefix check below it can never fire:

normalized_pattern = pattern.strip("/")   # "src/proto/" -> "src/proto"
...
if normalized_pattern.endswith("/"):      # never true after strip("/")
    return normalized.startswith(normalized_pattern)

A directory-style pattern such as proto/ (a natural GitHub-path-filter style
for the protocol-paths / conformance-paths inputs) falls through to
fnmatchcase(path, "proto"), which only matches a file literally named
proto. Reproduced:

>>> path_matches("src/proto/wire.go", "src/proto/")
False

The failure direction is the dangerous one: an SDK repo that configures
protocol-paths: proto/ gets protocol_changed=false for every PR touching
proto/, so protocol-sensitive changes silently bypass the conformance gate.
The same dead branch makes select_ci_adapters.py drop those files from
adapter selection and can make validate_conformance_pr.py reject valid
coverage PRs.

Fix

Record whether the pattern is directory-style before stripping slashes, and
match on the directory prefix explicitly:

is_dir_pattern = pattern.endswith("/")
normalized_pattern = pattern.strip("/")
...
if is_dir_pattern:
    return normalized.startswith(normalized_pattern + "/")

dir/ now matches every file under dir/ (recursively), does not match a
file literally named dir, and does not over-match sibling directories that
share the prefix (src/proto/ no longer risks matching src/protocol/…).
The /** and plain-glob branches are unchanged. All three copies were fixed
identically, and the if not normalized_pattern empty-pattern guard that only
check_conformance_policy.py had is now present in all three.

Testing

  • Added PathMatchesTest to test_check_conformance_policy.py, which
    exercises all three module copies (directory patterns, ** globs, exact
    paths, empty patterns) so the copies cannot silently drift again.
  • python3 scripts/test_check_conformance_policy.py — 7 tests pass
    (3 pre-existing subprocess tests + 4 new).

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.

1 participant