ADR-015: the site-literal check was matching nothing, and had been all along

The patterns are assembled from fragments so the published script does not
contain the strings it greps for. They used the apostrophe-escape form
('a'\''b'), which EMBEDS a literal apostrophe, instead of the concatenation form
('a''b'), which joins them. Every pattern therefore carried a stray quote in the
middle and could never match a line of real code, so the check passed everything
from the day it was written.

Verified the only way that means anything: plant a violation, watch it fail,
revert. Fixed, it immediately found 25 real hits.

Among them, in seven places, the default ServiceNow URLs - one organisation's QA
tenant, shipped with the integration ENABLED. Any site that had not configured
ServiceNow still rendered ticket links, pointing its technicians at a tenant
they cannot reach; an adopting site would have been pointed into somebody else's
instance entirely. There is no sensible cross-site default for "which ServiceNow
is yours", so the URLs ship blank and the integration ships off, matching the
Zabbix block that got this right.

A named SaaS tenant is as site-specific as a hostname, so it joins the patterns.
Colocated .spec.js files are excluded the same way tests/ already is: sample data
naming a facility is not a shipped literal.

tests/test_core/test_search_integrations.py

@@ -14,14 +14,41 @@ from shopdb.core.api.settings import invalidate_settings_cache
 from shopdb.core.api.search import _get_search_integrations, _classify_query
 
 
+def _configure_servicenow():
+    """Turn ServiceNow on with a tenant URL.
+
+    Required now, and that is the point: the integration ships OFF with a BLANK
+    URL, because the defaults used to be one organisation's QA tenant and any
+    adopting site inherited links into somebody else's ServiceNow.
+    """
+    Setting.set('servicenow_enabled', True, valuetype='boolean',
+                category='integrations')
+    Setting.set('servicenow_search_url',
+                'https://example.service-now.com/search/{ticket}',
+                valuetype='string', category='integrations')
+
+
 def _redirect(client, auth_headers, term):
     resp = client.get(f'/api/search?q={term}', headers=auth_headers)
     assert resp.status_code == 200, resp.get_json()
     return resp.get_json()['data'].get('redirect')
 
 
+def test_unconfigured_servicenow_yields_no_redirect(client, db, auth_headers):
+    """Shipped OFF and BLANK. Unconfigured must mean inactive.
+
+    The defaults were once a QA ServiceNow tenant with enabled=true, so a site
+    that had never configured ServiceNow still sent its technicians to a tenant
+    they could not reach - and an adopting site to another organisation's.
+    """
+    invalidate_settings_cache()
+    redirect = _redirect(client, auth_headers, 'GEINC123')
+    assert redirect is None or redirect.get('type') != 'servicenow'
+
+
 def test_default_prefix_classifies_servicenow(client, db, auth_headers):
-    """With no override, a GE-prefixed ticket redirects to ServiceNow."""
+    """Once configured, a default-prefixed ticket redirects to ServiceNow."""
+    _configure_servicenow()
     invalidate_settings_cache()
     redirect = _redirect(client, auth_headers, 'GEINC123')
     assert redirect is not None
@@ -30,6 +57,7 @@ def test_default_prefix_classifies_servicenow(client, db, auth_headers):
 
 def test_custom_prefix_classifies_servicenow(client, db, auth_headers):
     """A site-configured prefix classifies its tickets as ServiceNow."""
+    _configure_servicenow()
     Setting.set('servicenow_ticket_prefixes', 'ACMEINC', valuetype='string',
                 category='integrations')
     invalidate_settings_cache()

tests/test_core/test_settings_branding.py

@@ -143,12 +143,19 @@ def test_defaults_contain_new_branding_keys():
 def test_defaults_contain_new_integration_keys():
     """build_default_settings seeds the ServiceNow integration keys."""
     by_key = _defaults_by_key()
-    assert by_key['servicenow_enabled']['value'] == 'true'
+    # Ships OFF with blank URLs: the defaults were once a QA tenant, which an
+    # adopting site would have inherited as links into another organisation.
+    assert by_key['servicenow_enabled']['value'] == 'false'
     assert by_key['servicenow_enabled']['category'] == 'integrations'
     assert by_key['servicenow_ticket_prefixes']['value'] == 'GEINC,GECHG,GERIT,GESCT'
-    assert '{ticket}' in by_key['servicenow_search_url']['value']
-    assert '{ticket}' in by_key['servicenow_incident_url']['value']
-    assert '{ticket}' in by_key['servicenow_change_url']['value']
+    # Blank, not a tenant URL. There is no cross-site default for "which
+    # ServiceNow instance is yours", and a blank URL is what the search code
+    # reads as "not configured" - tickets then render as plain text.
+    for key in ('servicenow_search_url', 'servicenow_incident_url',
+                'servicenow_change_url'):
+        assert by_key[key]['value'] == '', key
+        assert '{ticket}' in by_key[key]['description'], \
+            'the placeholder must still be documented for whoever fills it in'
 
 
 def test_defaults_contain_new_site_keys():