From 0f14ff87d79b4e85e9ff74f2d95fe13b714ece7e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 16 Aug 2026 22:28:03 +0900 Subject: [PATCH] fix(scheduler): accept safe Unicode pull-request refs Replay the unique Git ref allowlist onto current main so GitHub-valid emoji and Hangul branches dispatch while shell and control characters stay rejected. --- .../strix-changed-path-quality-ci.yml | 3 +++ CHANGELOG.md | 1 + docs/doctoring/scheduler-unicode-git-refs.md | 24 +++++++++++++++++ scripts/ci/pr_review_merge_scheduler.py | 20 +++++++++++--- tests/test_pr_review_merge_scheduler.py | 11 +++++++- tests/test_strix_changed_path_policy.py | 26 +++++++++++++++++++ 6 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 docs/doctoring/scheduler-unicode-git-refs.md diff --git a/.github/workflows/strix-changed-path-quality-ci.yml b/.github/workflows/strix-changed-path-quality-ci.yml index 75e9b7d8e..bcb43eae1 100644 --- a/.github/workflows/strix-changed-path-quality-ci.yml +++ b/.github/workflows/strix-changed-path-quality-ci.yml @@ -13,6 +13,9 @@ on: - "tests/test_strix_changed_path_policy.py" - "tests/test_strix_workflow_dependency_hashes.py" - "tests/test_strix_quality_timeout_fixture_budget.py" + - ".github/workflows/pr-review-merge-scheduler.yml" + - "scripts/ci/pr_review_merge_scheduler.py" + - "tests/test_pr_review_merge_scheduler.py" permissions: contents: read diff --git a/CHANGELOG.md b/CHANGELOG.md index 1de9130a5..dae3637d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Accepted GitHub-valid Unicode pull-request refs (for example emoji and Hangul branch names) while still rejecting shell metacharacters, Unicode control/format/separator characters, and Git-unsafe path forms; scheduler source changes now trigger the exact-head full-suite quality gate. - Materialized base Python locks only when every package line is an exact SHA-256 pin or a bounded relative `-r`/`--requirement` include. A lone `--require-hashes` directive, a dotted include such as `./lock.txt`, or `-r other-hashes.txt` no longer enters the trusted build context. - Refused a conflict-scope repository root whose immediate parent is a symbolic link, so a swapped parent cannot redirect the canonical worktree after the last-component check (CWE-367). - Bounded the Strix quality self-test's deterministic timeout fixtures to 3-second process and 5-second fake-sleep budgets so exact-head policy evidence completes inside the existing job limit without changing production Strix scanner timeouts, providers, credentials, or review semantics. diff --git a/docs/doctoring/scheduler-unicode-git-refs.md b/docs/doctoring/scheduler-unicode-git-refs.md new file mode 100644 index 000000000..dafa0a5bc --- /dev/null +++ b/docs/doctoring/scheduler-unicode-git-refs.md @@ -0,0 +1,24 @@ +# Scheduler Unicode Git refs + +## Incident and buyer impact + +The merge scheduler rejected GitHub-valid branch names such as +`🎨-palette-ux-improvement-13325911538352561627` because `validate_git_ref` +used an ASCII-only regular expression. International product branches never +reached dispatch. + +## Decision + +Permit non-ASCII graphic and letter characters. Continue to reject ASCII +shell metacharacters and whitespace; Unicode control, format, and separator +categories; leading dashes; reserved `HEAD`; `@{`; traversal and hidden +components; trailing dots or slashes; and component `.lock` suffixes. All +Git and GitHub calls stay structured argv/API fields. + +## References + +Chacon, S., & Straub, B. (2014). *Pro Git* (2nd ed.). Apress. +https://git-scm.com/docs/git-check-ref-format + +The Unicode Consortium. (2024). *The Unicode Standard* (Version 16.0.0). +https://www.unicode.org/versions/Unicode16.0.0/ diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 75e18c860..dd12d47c0 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -12,6 +12,7 @@ import subprocess import sys import time +import unicodedata from collections.abc import Sequence from dataclasses import dataclass from datetime import datetime, timezone @@ -126,7 +127,7 @@ RUNNING_CHECK_STATES = {"PENDING", "EXPECTED", "QUEUED", "IN_PROGRESS", "WAITING", "REQUESTED"} FAILED_CHECK_CONCLUSIONS = {"FAILURE", "ERROR", "CANCELLED", "TIMED_OUT", "STARTUP_FAILURE"} ACTION_REQUIRED_CONCLUSIONS = {"ACTION_REQUIRED"} -GIT_REF_RE = re.compile(r"^(?!-)[A-Za-z0-9._/-]+$") +GIT_REF_ASCII_SAFE_CHARS = frozenset("._/-") GIT_SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") GITHUB_REPOSITORY_RE = re.compile(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") REVIEW_BODY_HEAD_SHA_RE = re.compile(r"Head SHA:\s*`([0-9a-fA-F]{40})`") @@ -530,10 +531,20 @@ def split_repo(repo: str) -> tuple[str, str]: def validate_git_ref(ref: str) -> str: """Return a conservative Git ref name for gh workflow dispatch fields.""" + has_unsafe_character = not isinstance(ref, str) or any( + ( + ord(character) < 128 + and not character.isalnum() + and character not in GIT_REF_ASCII_SAFE_CHARS + ) + or (ord(character) >= 128 and unicodedata.category(character)[0] in {"C", "Z"}) + for character in ref + ) if ( not isinstance(ref, str) or not ref - or not GIT_REF_RE.fullmatch(ref) + or has_unsafe_character + or ref.startswith("-") or ref == "HEAD" or ref.startswith("/") or ref.endswith(("/", ".")) @@ -542,7 +553,10 @@ def validate_git_ref(ref: str) -> str: or "//" in ref ): raise ValueError(f"invalid git ref: {ref!r}") - if any(part == "." or part.startswith(".") for part in ref.split("/")): + if any( + part == "." or part.startswith(".") or part.endswith(".lock") + for part in ref.split("/") + ): raise ValueError(f"invalid git ref: {ref!r}") return ref diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 3e421e903..f9685b35b 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -162,7 +162,12 @@ def test_run_split_repo_and_graphql(monkeypatch): with pytest.raises(ValueError): sched.split_repo("/repo") - assert sched.validate_git_ref("feature/safe.branch-1") == "feature/safe.branch-1" + for safe_ref in ( + "feature/safe.branch-1", + "🎨-palette-ux-improvement-13325911538352561627", + "κΈ°λŠ₯/μ•ˆμ „ν•œ-브랜치", + ): + assert sched.validate_git_ref(safe_ref) == safe_ref for bad_ref in ( "", "-bad", @@ -174,6 +179,10 @@ def test_run_split_repo_and_graphql(monkeypatch): "feature/main.", "feature/@{upstream}", "feat;echo pwned", + "feature/main.lock", + "feature/non\u00a0breaking-space", + "feature/zero\u200bwidth-space", + "feature/control\ncharacter", ): with pytest.raises(ValueError): sched.validate_git_ref(bad_ref) diff --git a/tests/test_strix_changed_path_policy.py b/tests/test_strix_changed_path_policy.py index 4d5ddd3c4..be3ec2cf0 100644 --- a/tests/test_strix_changed_path_policy.py +++ b/tests/test_strix_changed_path_policy.py @@ -11,6 +11,9 @@ REPOSITORY_ROOT = Path(__file__).resolve().parents[1] GATE_SCRIPT = REPOSITORY_ROOT / "scripts" / "ci" / "strix_quick_gate.sh" +QUALITY_WORKFLOW = ( + REPOSITORY_ROOT / ".github" / "workflows" / "strix-changed-path-quality-ci.yml" +) START_MARKER = 'python3 - "$REPO_ROOT" "$changed_file" <<\'PY\'\n' END_MARKER = "\nPY\n}\n\nnormalize_changed_files_cache()" LEGAL_PACKRAT_PATH = ( @@ -90,5 +93,28 @@ def test_rejects_traversal_absolute_controls_and_shell_punctuation(self) -> None self.assertEqual(result.stdout, "") +class MergeSchedulerQualityTriggerTests(unittest.TestCase): + """Keep scheduler control-plane changes inside an exact-head full-suite gate.""" + + def test_scheduler_source_workflow_and_tests_trigger_quality_ci(self) -> None: + """Every central merge-scheduler surface must trigger the permanent gate.""" + + workflow = QUALITY_WORKFLOW.read_text(encoding="utf-8") + for path in ( + ".github/workflows/pr-review-merge-scheduler.yml", + "scripts/ci/pr_review_merge_scheduler.py", + "tests/test_pr_review_merge_scheduler.py", + "tests/test_strix_changed_path_policy.py", + ): + self.assertEqual(workflow.count(f' - "{path}"'), 1) + + self.assertIn( + "ref: ${{ github.event.pull_request.head.sha || github.sha }}", + workflow, + ) + self.assertIn("python -m coverage run -m pytest tests -q", workflow) + self.assertIn("git diff --exit-code", workflow) + + if __name__ == "__main__": unittest.main()