Skip to content

Commit 8172e6f

Browse files
committed
fix(host): recognize review-app URLs as SaaS-like environments
Review-app hostnames (e.g. dashboard-23683.review-apps.preprod.gitguardian.tech) were treated as self-hosted, causing the client to call /exposed/v1 on the dashboard hostname instead of deriving the API hostname. The dashboard VirtualService has no /exposed/ route, so requests fell through to the frontend and returned HTML instead of JSON. Add suffix-based matching for .gitguardian.com and .gitguardian.tech domains so the client correctly derives the API hostname (api-23683.review-apps...) for these dynamic environments. Refs: APPAI-556
1 parent 122bfd6 commit 8172e6f

File tree

3 files changed

+46
-9
lines changed

3 files changed

+46
-9
lines changed

packages/gg_api_core/src/gg_api_core/host.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,13 @@
1010
"dashboard.preprod.gitguardian.com",
1111
]
1212

13+
# Domain suffixes for dynamic SaaS-like environments (e.g. review-apps)
14+
# where hostnames include a dynamic ID like dashboard-23683.review-apps.preprod.gitguardian.tech
15+
SAAS_DOMAIN_SUFFIXES = [
16+
".gitguardian.com",
17+
".gitguardian.tech",
18+
]
19+
1320
LOCAL_HOSTNAMES = ["localhost", "127.0.0.1"]
1421

1522

@@ -20,6 +27,14 @@ def _is_local_hostname(parsed_hostname: str | None) -> bool:
2027
return parsed_hostname.lower() in LOCAL_HOSTNAMES
2128

2229

30+
def _is_saas_hostname(netloc: str) -> bool:
31+
"""Check if a netloc (host:port) belongs to a SaaS or SaaS-like environment."""
32+
if netloc in SAAS_HOSTNAMES:
33+
return True
34+
# Match dynamic environments like review-apps (e.g. dashboard-23683.review-apps.preprod.gitguardian.tech)
35+
return any(netloc.endswith(suffix) for suffix in SAAS_DOMAIN_SUFFIXES)
36+
37+
2338
def is_self_hosted_instance(gitguardian_url: str | None = None) -> bool:
2439
"""
2540
Determine if we're connecting to a self-hosted GitGuardian instance.
@@ -40,7 +55,7 @@ def is_self_hosted_instance(gitguardian_url: str | None = None) -> bool:
4055
return False
4156
# For SaaS, check the full netloc (includes port)
4257
netloc = parsed.netloc.lower()
43-
return netloc not in SAAS_HOSTNAMES
58+
return not _is_saas_hostname(netloc)
4459
except Exception:
4560
# If parsing fails, assume self-hosted to be safe
4661
return True

tests/test_client.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
@pytest.fixture
1010
def client():
1111
"""Fixture to create a client instance with OAuth authentication."""
12-
with patch.dict(os.environ, {"GITGUARDIAN_URL": "https://test.gitguardian.com"}):
12+
with patch.dict(os.environ, {"GITGUARDIAN_URL": "https://gitguardian.mycompany.com"}):
1313
client = GitGuardianClient()
1414
# Mock the OAuth token to prevent OAuth flow during tests
1515
client._oauth_token = "test_oauth_token"
@@ -41,7 +41,7 @@ class TestGitGuardianClient:
4141

4242
def test_init_with_env_vars(self, client):
4343
"""Test client initialization with environment variables."""
44-
assert client.public_api_url == "https://test.gitguardian.com/exposed/v1"
44+
assert client.public_api_url == "https://gitguardian.mycompany.com/exposed/v1"
4545

4646
def test_init_with_params(self):
4747
"""Test client initialization with parameters."""

tests/test_host.py

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import os
44
from unittest.mock import patch
55

6-
from gg_api_core.host import SAAS_HOSTNAMES, is_self_hosted_instance
6+
from gg_api_core.host import SAAS_HOSTNAMES, _is_saas_hostname, is_self_hosted_instance
77

88

99
class TestIsSelfHostedInstance:
@@ -104,16 +104,38 @@ def test_url_with_different_ports(self):
104104

105105
def test_url_with_basic_auth(self):
106106
"""Test that URLs with basic auth credentials are handled correctly."""
107-
# Note: urlparse includes credentials in netloc (e.g., "user:pass@hostname")
108-
# Since "user:pass@dashboard.gitguardian.com" is not in SAAS_HOSTNAMES,
109-
# the function returns True (treats it as self-hosted)
110-
# This is an edge case where the current implementation doesn't strip credentials
111-
assert is_self_hosted_instance("https://user:pass@dashboard.gitguardian.com") is True
107+
# urlparse includes credentials in netloc (e.g., "user:pass@hostname")
108+
# The suffix-based matching still recognizes the SaaS domain
109+
assert is_self_hosted_instance("https://user:pass@dashboard.gitguardian.com") is False
112110
assert is_self_hosted_instance("https://user:pass@custom.domain.com") is True
113111

112+
def test_review_app_returns_false(self):
113+
"""Test that review-app URLs are recognized as SaaS-like (not self-hosted)."""
114+
review_app_urls = [
115+
"https://dashboard-23683.review-apps.preprod.gitguardian.tech",
116+
"https://api-23683.review-apps.preprod.gitguardian.tech",
117+
"https://dashboard-99999.review-apps.preprod.gitguardian.tech",
118+
]
119+
for url in review_app_urls:
120+
assert is_self_hosted_instance(url) is False, f"Expected False for {url}"
121+
114122
def test_all_saas_hostnames_in_list(self):
115123
"""Test that all hostnames in SAAS_HOSTNAMES are correctly identified."""
116124
for hostname in SAAS_HOSTNAMES:
117125
# Construct a full URL for each hostname
118126
url = f"https://{hostname}"
119127
assert is_self_hosted_instance(url) is False, f"Expected False for {url}"
128+
129+
130+
class TestIsSaasHostname:
131+
"""Tests for the _is_saas_hostname function."""
132+
133+
def test_exact_match(self):
134+
assert _is_saas_hostname("dashboard.gitguardian.com") is True
135+
136+
def test_suffix_match(self):
137+
assert _is_saas_hostname("dashboard-23683.review-apps.preprod.gitguardian.tech") is True
138+
139+
def test_custom_domain_not_matched(self):
140+
assert _is_saas_hostname("gitguardian.mycompany.com") is False
141+
assert _is_saas_hostname("gg.internal.corp") is False

0 commit comments

Comments
 (0)