mirror of
https://github.com/github/spec-kit.git
synced 2026-08-15 14:02:04 +02:00
fix(auth): treat exact host patterns literally (#4108)
Assisted-by: OpenAI Codex (model: GPT-5, autonomous)
This commit is contained in:
@@ -11,7 +11,6 @@ import json
|
||||
import os
|
||||
import stat
|
||||
from dataclasses import dataclass
|
||||
from fnmatch import fnmatch
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from urllib.parse import urlparse
|
||||
@@ -48,10 +47,21 @@ def _is_valid_host_pattern(pattern: str) -> bool:
|
||||
* ``*.example.com`` — leading ``*.`` wildcard; matches subdomains
|
||||
such as ``myorg.example.com`` but not ``example.com`` itself
|
||||
"""
|
||||
if any(char in pattern for char in "?[]"):
|
||||
return False
|
||||
if "*" not in pattern:
|
||||
return True # exact hostname — already validated as non-empty
|
||||
# Only *.suffix is allowed; no other wildcard positions
|
||||
return pattern.startswith("*.") and "*" not in pattern[2:]
|
||||
return pattern.startswith("*.") and len(pattern) > 2 and "*" not in pattern[2:]
|
||||
|
||||
|
||||
def _host_matches_pattern(hostname: str, pattern: str) -> bool:
|
||||
"""Match a hostname against an exact host or leading ``*.`` wildcard."""
|
||||
hostname = hostname.lower()
|
||||
pattern = pattern.lower()
|
||||
if pattern.startswith("*.") and _is_valid_host_pattern(pattern):
|
||||
return hostname.endswith(pattern[1:])
|
||||
return hostname == pattern
|
||||
|
||||
|
||||
def _norm(value: Any) -> Any:
|
||||
@@ -224,8 +234,5 @@ def find_entries_for_url(
|
||||
return [
|
||||
e
|
||||
for e in entries
|
||||
if any(
|
||||
pattern == hostname or fnmatch(hostname, pattern)
|
||||
for pattern in e.hosts
|
||||
)
|
||||
if any(_host_matches_pattern(hostname, pattern) for pattern in e.hosts)
|
||||
]
|
||||
|
||||
@@ -13,13 +13,18 @@ from __future__ import annotations
|
||||
|
||||
import urllib.error
|
||||
import urllib.request
|
||||
from fnmatch import fnmatch
|
||||
from typing import Callable
|
||||
from urllib.parse import urlparse
|
||||
|
||||
from .._download_security import is_safe_download_redirect
|
||||
from . import get_provider
|
||||
from .config import AuthConfigEntry, _default_config_path, find_entries_for_url, load_auth_config
|
||||
from .config import (
|
||||
AuthConfigEntry,
|
||||
_default_config_path,
|
||||
_host_matches_pattern,
|
||||
find_entries_for_url,
|
||||
load_auth_config,
|
||||
)
|
||||
|
||||
|
||||
_config_override: list[AuthConfigEntry] | None = None
|
||||
@@ -54,8 +59,7 @@ def _load_config() -> list[AuthConfigEntry]:
|
||||
|
||||
def _hostname_in_hosts(hostname: str, hosts: tuple[str, ...]) -> bool:
|
||||
"""Return True if *hostname* matches any pattern in *hosts*."""
|
||||
hostname = hostname.lower()
|
||||
return any(p == hostname or fnmatch(hostname, p) for p in hosts)
|
||||
return any(_host_matches_pattern(hostname, pattern) for pattern in hosts)
|
||||
|
||||
|
||||
RedirectValidator = Callable[[str, str], None]
|
||||
|
||||
@@ -302,6 +302,20 @@ class TestLoadAuthConfig:
|
||||
with pytest.raises(ValueError, match="invalid host pattern"):
|
||||
load_auth_config(cfg)
|
||||
|
||||
@pytest.mark.parametrize("host", ["gith?b.com", "[a-z].example.com"])
|
||||
def test_unsupported_glob_metacharacters_raise(self, tmp_path, host):
|
||||
cfg = tmp_path / "auth.json"
|
||||
cfg.write_text(json.dumps({
|
||||
"providers": [{
|
||||
"hosts": [host],
|
||||
"provider": "github",
|
||||
"auth": "bearer",
|
||||
"token_env": "X",
|
||||
}]
|
||||
}))
|
||||
with pytest.raises(ValueError, match="invalid host pattern"):
|
||||
load_auth_config(cfg)
|
||||
|
||||
def test_valid_star_dot_host_accepted(self, tmp_path):
|
||||
cfg = tmp_path / "auth.json"
|
||||
cfg.write_text(json.dumps({
|
||||
@@ -344,6 +358,39 @@ class TestFindEntriesForUrl:
|
||||
result = find_entries_for_url("https://myorg.visualstudio.com/project", [entry])
|
||||
assert result == [entry]
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"url",
|
||||
[
|
||||
"https://visualstudio.com/project",
|
||||
"https://evilvisualstudio.com/project",
|
||||
"https://visualstudio.com.evil.example/project",
|
||||
],
|
||||
)
|
||||
def test_wildcard_does_not_match_apex_or_lookalikes(self, url):
|
||||
entry = AuthConfigEntry(
|
||||
hosts=("*.visualstudio.com",),
|
||||
provider="azure-devops",
|
||||
auth="basic-pat",
|
||||
token_env="ADO_PAT",
|
||||
)
|
||||
assert find_entries_for_url(url, [entry]) == []
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("pattern", "url"),
|
||||
[
|
||||
("gith?b.com", "https://github.com/org/repo"),
|
||||
("[a-z].example.com", "https://a.example.com/file"),
|
||||
],
|
||||
)
|
||||
def test_exact_hosts_do_not_apply_glob_semantics(self, pattern, url):
|
||||
entry = AuthConfigEntry(
|
||||
hosts=(pattern,),
|
||||
provider="github",
|
||||
auth="bearer",
|
||||
token="sentinel",
|
||||
)
|
||||
assert find_entries_for_url(url, [entry]) == []
|
||||
|
||||
def test_no_match_returns_empty(self):
|
||||
entry = _github_entry()
|
||||
result = find_entries_for_url("https://evil.example.com/file", [entry])
|
||||
@@ -1049,6 +1096,39 @@ class TestRedirectStripping:
|
||||
assert new_req.headers.get("Authorization") is None
|
||||
assert new_req.unredirected_hdrs.get("Authorization") is None
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("hosts", "target", "expected_auth"),
|
||||
[
|
||||
(("*.example.com",), "https://api.example.com/asset", "Bearer tok"),
|
||||
(("*.example.com",), "https://example.com/asset", None),
|
||||
(("*.example.com",), "https://evil-example.com/asset", None),
|
||||
(("gith?b.com",), "https://github.com/asset", None),
|
||||
(("[a-z].example.com",), "https://a.example.com/asset", None),
|
||||
],
|
||||
)
|
||||
def test_redirect_host_patterns_use_literal_safe_matching(
|
||||
self, hosts, target, expected_auth
|
||||
):
|
||||
from specify_cli.authentication.http import _StripAuthOnRedirect
|
||||
from urllib.request import Request
|
||||
import io
|
||||
|
||||
handler = _StripAuthOnRedirect(hosts)
|
||||
req = Request(
|
||||
"https://source.example.org/file",
|
||||
headers={"Authorization": "Bearer tok"},
|
||||
)
|
||||
new_req = handler.redirect_request(
|
||||
req, io.BytesIO(b""), 302, "Found", {}, target
|
||||
)
|
||||
|
||||
assert new_req is not None
|
||||
auth = (
|
||||
new_req.get_header("Authorization")
|
||||
or new_req.unredirected_hdrs.get("Authorization")
|
||||
)
|
||||
assert auth == expected_auth
|
||||
|
||||
def test_https_to_http_same_host_redirect_rejected(self):
|
||||
from specify_cli.authentication.http import _StripAuthOnRedirect
|
||||
from urllib.request import Request
|
||||
|
||||
Reference in New Issue
Block a user