PR #5: ServiceNow MCP server

[saengland]

Adds the ServiceNow MCP server under solutions/ess-agent-kit/src/mcp/servicenow/. Local stdio MCP server that proxies REST API calls to the customer's ServiceNow tenant.

SFI focus areas:

• OAuth2 token handling (refresh, expiry, storage)
• REST request construction and URL handling
• Network egress (only customer-provided ServiceNow endpoint)
• Error/response handling (no leakage of customer ticket data)

Tracker: #10

[Graham McMynn (GrahamMcMynn)]

Code review by ESS Code Review Agent (4 comments)

[Graham McMynn (GrahamMcMynn)]

Bug: 429 retry exhaustion gives misleading None error

When all retry attempts receive HTTP 429 responses, the loop falls through to raise Exception(f"Max retries exceeded: {last_error}"). But last_error is only assigned inside the except httpx.RequestError block — not in the 429 branch. So if every attempt is rate-limited, the error message will be "Max retries exceeded: None", which is misleading and hides the real cause.

Suggested fix: Set last_error in the 429 branch too:

if resp.status_code == 429:
    last_error = Exception(
        f"Rate limited (429), Retry-After={resp.headers.get('Retry-After')}"
    )
    wait = int(resp.headers.get("Retry-After", str(2**attempt)))
    ...

[saengland]

Fixed in 831d343. Set last_error inside the 429 branch with a Rate limited (429), Retry-After=... Exception so Max retries exceeded reports the actual cause when every attempt is rate-limited.

[Graham McMynn (GrahamMcMynn)]

Error handling: Unhandled json.loads across multiple tools

json.loads(data) is called without error handling here, in update_record (line 125), log_copilot_summary (line 503), and call_api (line 722). Since these string parameters come from LLM tool calls, malformed JSON will produce an unhandled JSONDecodeError with a raw Python traceback instead of a user-friendly MCP error.

Suggested fix: Wrap each json.loads in a try/except, or extract a helper:

def _parse_json(raw: str, field_name: str) -> dict:
    try:
        return json.loads(raw)
    except json.JSONDecodeError as e:
        raise ValueError(f"Invalid JSON in '{field_name}': {e}") from e

[saengland]

Fixed in 831d343. Added a _parse_json(raw, field_name) helper and wired it into create_record, update_record, log_copilot_summary, and call_api. Tracebacks no longer leak; tools return a friendly ValueError the LLM can self-correct from.

[Graham McMynn (GrahamMcMynn)]

Security: call_api is an unrestricted proxy to any ServiceNow REST endpoint

This tool allows calling any REST path on the ServiceNow instance with any HTTP method. Combined with set_system_property and delete_record, this gives the LLM the ability to call destructive internal APIs (e.g., disable authentication, drop records, invoke admin scripted REST APIs) — especially concerning given SFI is a stated focus area of this PR.

Additionally, method accepts any string with no validation.

Suggested fix (at least one of):

1. Path allowlist — restrict to known safe prefixes (e.g., /api/now/table/, /api/now/stats/)
2. Method allowlist — only allow GET, POST, PATCH, DELETE
3. Admin-API denylist — block known dangerous paths
4. Remove call_api entirely — the typed tools already cover the intended use cases

[saengland]

Fixed in 831d343. Implemented A + B + C combined:

• Path allowlist: call_api now requires the path to start with one of /api/now/table/, /api/now/stats/, /api/now/import/, /api/now/attachment, /api/now/v1/table/, /api/now/v2/table/. Admin/scripting paths are blocked.
• Method allowlist: only GET, POST, PATCH, DELETE. PUT, OPTIONS, HEAD and anything custom are rejected.
• Documentation: docstring marks the tool as EXPLORATION ONLY and points users to the typed tools (query_table, get_record, etc.) for supported scenarios.

[Graham McMynn (GrahamMcMynn)]

Security: OAuth/OIDC secrets passed as plaintext tool parameters

register_oauth_application and register_oidc_provider accept client_secret as a plain-text tool parameter. MCP tool invocations are typically logged (in VS Code output, MCP inspector, etc.), which means secrets will appear in plaintext in logs and in the LLM's context window. The PR description flags OAuth2 token handling as an SFI focus area.

Suggested fix: Read the secret from an environment variable or secure reference instead:

client_secret_env: str = "SERVICENOW_OAUTH_CLIENT_SECRET"
# then: secret = os.environ[client_secret_env]

Alternatively, document that this tool is intended for initial setup only and that secrets will be visible in MCP logs.

[saengland]

Fixed in 831d343. Implemented env var indirection (Option A):

• register_oauth_application and register_oidc_provider now accept client_secret_env_var: str (the NAME of an env var) instead of client_secret (the raw value).
• New _resolve_secret_from_env(env_var, field_name) helper reads the secret from os.environ at execution.
• The secret never crosses the MCP tool boundary, so it doesn't appear in MCP logs or LLM context.

Note: 4 skill docs in PR #6 (step2-oauth2.md, step2-entra.md, step2-certificate.md, step2-graph.md) reference the old client_secret parameter shape. Flagged in the commit message; will be updated as a follow-up either in PR #6 or post-merge.

[saengland]

Follow-up on the env-var contract change: the 4 skill docs in PR #6 that called the old client_secret parameter shape have been updated to use the new client_secret_env_var contract. Pushed in 94b4667 on the review/06-skills-connect-onboarding branch (PR #16).

• step2-oauth2.md writes the freshly-generated CLIENT_SECRET to \ and passes that env var name to register_oauth_application.
• step2-certificate.md, step2-entra.md, step2-graph.md set a sentinel \ = 'not-used' and pass that name to register_oidc_provider (OIDC verification with Entra ID doesn't actually consume the secret, but the ServiceNow record requires the field to be non-empty).

[John Nguyen (johnguy0)]

Same author, same shape as PR #14, with most of the same defects plus two new CRITICAL ones specific to ServiceNow. The good things from PR #14 carry over (env-var creds, httpx.BasicAuth, retry with Retry-After, requirements.txt). What's different here is the size of the MCP tool surface (29 tools, including 9 admin/OAuth/system-property tools that I'd argue shouldn't exist as LLM-callable at all), and the ServiceNow encoded-query injection that doesn't have a Workday equivalent.

CRITICAL:

1. ServiceNow query injection across every search_ tool.* Lines 204, 313, 383, 421, 462, 502, 560, 634, 731. User-controlled input (query, state, priority, assigned_to, category, name) is concatenated into encoded queries with ^ as the separator. The ^ and ^OR operators are not escaped. An LLM-injected query="x^OR1=1" returns every record in the table. state="1^ORnumberSTARTSWITHINC" returns every incident. This is the SQL-injection equivalent for ServiceNow, and it's wide open. Fix: build an encoded query via a structured builder that URL-encodes values and rejects raw ^ in untrusted input. Don't pass encoded_query as a free string when individual filter parts are caller-controlled.

2. No HTTPS validation on SERVICENOW_INSTANCE_URL (client.py:34). Combined with httpx.BasicAuth, an http:// URL sends username + password in cleartext. Validate explicitly: if not instance_url.lower().startswith("https://"): raise ValueError(...). Same shape as PR #14 comment, even more important here because there's no fallback to digest or SAML.

HIGH:

3. Same five hardening defects as PR #14, listed inline. client.py:40 no follow_redirects=False; client.py:37 credentials as plain instance attribute; client.py:83-87 ServiceNow error message echoes customer data into LLM context; client.py:12 httpx logger not silenced; client.py:93 retry has no jitter. See inline comments. The fixes are the same as PR #14 and should be coordinated as one shared pattern across the two MCP servers.

4. Path traversal via table and sys_id parameters. client.py:127, 135, 140, 145, 150 all interpolate user-controlled strings into the URL path: f"/api/now/table/{table}/{sys_id}". If an LLM passes sys_id="abc/sys_user/admin-id", the path becomes /api/now/table/incident/abc/sys_user/admin-id which is a different endpoint. httpx does not auto-escape path segments. Validate: both table and sys_id must match ^[a-zA-Z0-9_]+$ (sys_ids are 32-char hex). Reject anything else.

5. Admin tools should not exist as LLM-callable. register_oauth_application (server.py:572), register_oidc_provider (server.py:644), set_system_property (server.py:741), delete_record (server.py:166). These are admin-equivalent operations. A prompt-injected LLM can register an attacker-controlled OAuth app with their own redirect_uri (token theft primitive), modify system properties to disable security controls, or delete arbitrary records. Even with the env-var-pass-by-name pattern for client_secret_env_var (which is a clever design and I'll note that), the LLM still controls all the other fields including redirect_url. Move these out of the tool surface. If they're needed for the /connect skill, expose them as a script that the human runs explicitly, not as an LLM-callable tool.

6. call_api allowlist is structurally correct but data-permissive. Method and path prefix are allowlisted (server.py:781-808), and admin endpoints (/api/now/auth/*, /api/now/sys_* direct access) are blocked. Good. But the allowlist includes /api/now/table/, which means the LLM can still DELETE /api/now/table/sys_user/<admin-sys-id> to remove a user account, or PATCH /api/now/table/sys_properties/<security-property-id> to modify a security setting. The allowlist gates the URL surface but not the tables those URLs target. Add a table-name denylist (sys_user, sys_properties, sys_security_, oauth_entity, sys_oidc_, etc.) for non-GET methods.

MEDIUM:

7. No write-operation confirmation. create_record, update_record, delete_record, create_incident, resolve_incident, create_hr_case, register_oauth_application, register_oidc_provider, set_system_property. Same shape as PR #14's request_time_off. Same fix: split into preview/confirm tools, or strong tool-description warnings, or move out of the tool surface entirely (preferred for #5 above).

8. Full ServiceNow responses returned to LLM via _fmt (server.py:42-53). HR cases and incidents include comments, descriptions, and reference fields that can carry SSN/employee personal data. Field allowlisting per tool is the right call.

9. No domain validation on SERVICENOW_INSTANCE_URL. Pair with the HTTPS check in #2: validate hostname matches *.service-now.com or a customer-configured allowlist. Defense-in-depth against env tampering.

10. _request opens a NEW httpx client per request (client.py:54), inside the retry loop. Every retry creates a fresh connection, fresh TLS handshake, fresh DNS. Reuse a single AsyncClient as an instance attribute, opened in __init__ (or a context manager), and use it across all calls. Performance and correctness both improve.

PR-level:

11. PR description claims "OAuth2 token handling" as an SFI focus area, but client.py uses only httpx.BasicAuth. There is no OAuth flow to ServiceNow in this code. The OAuth-related tools (register_oauth_application, register_oidc_provider, etc.) manage OAuth applications inside ServiceNow — they don't authenticate the MCP server to ServiceNow. The PR description is misleading about the auth model.

12. Stale solutions/ess-agent-kit/ path in description (same as PRs #2, #3, #4).

13. No python-dotenv in requirements.txt (PR #14 has it). If .vscode/mcp.json provides env vars via ${input:...}, that's fine — confirm. If anyone expects .env file support, this needs the dep.

Inline below.

[John Nguyen (johnguy0)]

CRITICAL — ServiceNow encoded-query injection.

query is concatenated into the filter as short_descriptionLIKE{query}. ServiceNow encoded queries use ^ as the AND separator and ^OR for OR. None of these are escaped.

LLM-injected payloads:

• query="x^OR1=1" → short_descriptionLIKEx^OR1=1 returns every record
• query="x^ORassigned_to=<other-user-sys-id>" exfiltrates incidents assigned to a different user

Same pattern at lines 313, 383, 421, 462, 502, 560, 634, 731. Every search_* tool is affected.

Fix: build the encoded query via a structured builder that rejects/escapes ^ and the operator keywords (^OR, ^NQ, ^EQ, ^LIKE, ^ENDSWITH, ^STARTSWITH, ^CONTAINS, ORDERBY, GROUPBY) when those characters appear in caller-supplied values:

import re
_FORBIDDEN_QUERY_CHARS = re.compile(r"[\^]")

def _safe_value(v: str) -> str:
    if _FORBIDDEN_QUERY_CHARS.search(v):
        raise ValueError(f"Invalid character in query value: {v!r}")
    return v

Then parts.append(f"short_descriptionLIKE{_safe_value(query)}") everywhere. State/priority/category/assigned_to should additionally be regex-validated to known formats (numeric for state/priority, sys_id format for assigned_to).

[John Nguyen (johnguy0)]

CRITICAL — no HTTPS validation; combined with Basic Auth, http://-misconfigured URL leaks credentials in cleartext.

Fix:

if not instance_url.lower().startswith("https://"):
    raise ValueError("SERVICENOW_INSTANCE_URL must use HTTPS")
self.base_url = instance_url.rstrip("/")

MEDIUM (paired) — add domain allowlist.

Validate the host matches *.service-now.com or a customer-configured allowlist. Without a domain check, an attacker who can write to env vars or .vscode/mcp.json can point the MCP server at their own host with a valid TLS cert and steal Basic Auth credentials.

from urllib.parse import urlparse
host = (urlparse(instance_url).hostname or "").lower()
if not host.endswith(".service-now.com"):
    raise ValueError(f"SERVICENOW_INSTANCE_URL must point to *.service-now.com (got {host})")

Accept an env-var override (SERVICENOW_ALLOWED_HOSTS) for customers on custom domains.

[John Nguyen (johnguy0)]

HIGH — same defect as PR #14: credentials as plain instance attribute.

self._auth = httpx.BasicAuth(username, password) — httpx.BasicAuth stores both as plain attributes. repr(client) or any traceback that includes the client object exposes them.

Fix: scrub via __repr__:

def __repr__(self):
    return f"ServiceNowClient(base_url={self.base_url!r})"

See my comment on PR #14 client.py:78 for the same pattern.

[John Nguyen (johnguy0)]

HIGH — three fixes in one place.

1. follow_redirects=False. httpx 0.27+ defaults to True. Combined with the BasicAuth sticking on the client, a 302 to an attacker host replays Authorization to the redirect target. Same defect as PR PR #4: Workday MCP server #14.

2. verify=True explicit. Defense-in-depth against REQUESTS_CA_BUNDLE env tampering. Same as PR PR #4: Workday MCP server #14.

3. Open the client once, reuse across all calls. Currently _request builds a fresh AsyncClient per call (line 54), which means every retry burns a new TLS handshake. Move client construction to __init__ (or to an async __aenter__/__aexit__) and reuse it.

Combined fix:

def __init__(self):
    ...
    self._client = httpx.AsyncClient(
        base_url=self.base_url,
        auth=self._auth,
        headers={"Accept": "application/json", "Content-Type": "application/json"},
        timeout=self.timeout,
        follow_redirects=False,
        verify=True,
    )

async def aclose(self):
    await self._client.aclose()

Then _request uses self._client.request(method, path, ...) directly. Caller (server.get_client()) becomes responsible for closing on shutdown.

[John Nguyen (johnguy0)]

HIGH — silence httpx loggers. Same defect as PR #14 client.py:20.

If anyone enables Python DEBUG logging, httpx logs full HTTP requests including the Authorization header (which is Basic <base64(user:pass)>). Add to module scope or __init__:

logging.getLogger("httpx").setLevel(logging.WARNING)
logging.getLogger("httpcore").setLevel(logging.WARNING)

[John Nguyen (johnguy0)]

HIGH — path traversal via table and sys_id.

This line and lines 127, 140, 145, 150 all interpolate user-controlled strings into the URL path with f-strings: f"/api/now/table/{table}/{sys_id}". httpx does not auto-escape path segments.

LLM-injected sys_id="abc/sys_user/admin-id" produces path /api/now/table/incident/abc/sys_user/admin-id — a different endpoint. Combined with the delete_record tool exposed at server.py:166, this is a path-traversal-to-arbitrary-delete primitive.

Fix: validate both arguments before interpolation.

import re
_TABLE_RE = re.compile(r"^[a-z][a-z0-9_]{0,63}$")
_SYS_ID_RE = re.compile(r"^[0-9a-f]{32}$")

def _validate_table(t):
    if not _TABLE_RE.match(t):
        raise ValueError(f"Invalid table name: {t!r}")
    return t

def _validate_sys_id(sid):
    if not _SYS_ID_RE.match(sid):
        raise ValueError(f"Invalid sys_id: {sid!r}")
    return sid

Apply to every CRUD method on the client. Tighten the table name regex against ServiceNow's actual table-naming rules if needed.

[John Nguyen (johnguy0)]

MEDIUM — _fmt returns full ServiceNow responses to the LLM unfiltered.

Every tool ends with return _fmt(result). ServiceNow responses for HR cases, incidents, and user lookups can include comments, descriptions, employee personal info, manager chains, and reference fields with sensitive data. All of it lands in the LLM conversation context (and potentially in LLM provider telemetry).

Two paths forward:

1. Field allowlist per tool. Each search_*/get_* tool already passes a fields parameter to query_table. Make that allowlist explicit and minimal (sys_id, number, short_description, state) and document field-extension as a customer-controlled config.
2. Filter in _fmt based on the table. Take a table argument and apply per-table redaction (e.g., for sn_hr_core_case, drop description and comments unless explicitly requested).

Go with #1 — it's already mostly in place, just needs to be the contract rather than the suggestion.

[John Nguyen (johnguy0)]

HIGH — delete_record exposes arbitrary table deletion to the LLM.

Combined with the path traversal in client.py:135, this is a primitive for deleting any record in any table. A prompt-injected LLM can call delete_record(table="sys_user", sys_id="<admin>") or delete_record(table="sys_audit", sys_id="<recent-audit-id>") to wipe audit records.

This tool should not be in the LLM-callable surface. If the kit needs delete capability, expose it as a script the human runs explicitly, not as a tool.

If you keep it: at minimum, denylist tables by name (sys_user, sys_audit, sys_security_, sys_properties, oauth_entity, sys_oidc_, sys_user_role, sys_user_grmember, sys_user_group), require a typed confirmation token from a separate prepare_delete tool, and log the action.

[John Nguyen (johnguy0)]

HIGH — admin tools should not be LLM-callable.

register_oauth_application (this tool), register_oidc_provider (line 644), and set_system_property (line 741) are admin-equivalent operations. The client_secret_env_var indirection here is clever and prevents secret leakage into LLM context — credit for that. But the LLM still controls every other field, including redirect_url. A prompt-injected LLM can register an attacker-controlled OAuth app with the attacker's redirect_uri and use the resulting flow to harvest tokens from real users.

These belong in the /connect skill (PR #6), invoked by a script the human explicitly runs after reading what's about to happen. They don't belong in the LLM tool surface.

My strong recommendation: remove from server.py. Move the implementation into solutions/ess-maker-skills/scripts/connect_servicenow.py and have the connect skill walk the human through the parameters before calling it.

If there's a hard reason to keep these in the MCP surface, add prominent warnings to the docstrings, gate them behind an env-var feature flag (SERVICENOW_MCP_ENABLE_ADMIN_TOOLS=1) that defaults off, and log every invocation locally.

[John Nguyen (johnguy0)]

HIGH — allowlist is structurally correct but data-permissive.

The method allowlist (GET, POST, PATCH, DELETE) and path-prefix allowlist (lines 793-808) are good — admin endpoints (/api/now/auth/, direct /api/now/sys_*) are correctly blocked. But the path allowlist includes /api/now/table/, which means the LLM can call:

• call_api("DELETE", "/api/now/table/sys_user/<admin-sys-id>") — delete admin user
• call_api("PATCH", "/api/now/table/sys_properties/<security-property-id>", data='{"value":"false"}') — disable a security control
• call_api("POST", "/api/now/table/sys_user_grmember", data='{"user":"<attacker>","group":"<admin-group>"}') — escalate privileges

The allowlist gates URL structure. It doesn't gate which tables those URLs target.

Fix: add a table denylist for non-GET methods on the table API. Concretely, parse the path; if it matches /api/now/(v1/|v2/)?table/(<table>), check <table> against a denylist of admin/security/audit tables before allowing non-GET methods.

_ADMIN_TABLES = {
    "sys_user", "sys_user_grmember", "sys_user_group", "sys_user_role",
    "sys_properties", "sys_audit", "sys_security_acl",
    "oauth_entity", "sys_oidc_provider", "sys_security_diag",
    "sys_script", "sys_script_include", "sys_script_action",
}

The spirit of the allowlist (EXPLORATION ONLY, per the docstring) is right. Make the implementation match the intent.

[saengland]

Addressed all CRITICAL+HIGH+MEDIUM findings in 5df862b.

CRITICAL: Encoded query injection (9 search tools, 20 unsafe interpolations)

• Added _q() helper that strips ^ (encoded-query AND), ^OR, null, CR, LF from values before they're concatenated into a ^-joined query.
• Wired _q() through every parts.append(...) site:
  • search_incidents (5 fields: query, state, priority, assigned_to, category)
  • search_hr_cases (4: query, state, hr_service, opened_for)
  • browse_service_catalog (2: query, category)
  • search_cmdb_items (2: query, operational_status)
  • resolve_user (4: name, email, user_name, employee_number)
  • search_interactions (3: query, state, channel)
• Total: 20 injection sites closed. Prompt-injection inputs like query="x^OR1=1" (tautology) or query="x^ORassigned_to=admin" (cross-user data leak) now have the ^ replaced with a space and the rest of the value goes into the LIKE/equals clause as intended.

HIGH: Path traversal in get_record/update_record/delete_record (sys_id interpolation)

• Added strict regex validation in client.py:
  • _TABLE_NAME_RE = ^[a-z][a-z0-9_]{0,63}$
  • _SYS_ID_RE = ^[0-9a-fA-F]{32}$
• Inputs like sys_id="abc/sys_user/admin-id" now raise ValueError before they can compose a different REST path.

HIGH: delete_record on protected tables

• Added _DENYLIST_TABLES: sys_user, sys_user_group, sys_user_role, sys_audit, sys_audit_delete, sys_log, sys_log_transaction, sys_security_acl, sys_properties, oauth_entity, oauth_entity_profile, sys_certificate.
• delete_record / update_record / create_record reject these. query_table / get_record / get_stats still allow read access (otherwise we couldn't resolve_user a sys_user).

HIGH: HTTPS enforcement

• SERVICENOW_INSTANCE_URL must start with https:// or ServiceNowClient() raises ValueError. Refuses to send credentials over cleartext.

HIGH: httpx DEBUG credential exposure

• httpx and httpcore loggers explicitly silenced to WARNING.

HIGH: follow_redirects=True default

• _build_client() now sets follow_redirects=False. A 302 cannot replay the Authorization header to an attacker-controlled host.

HIGH: Plain password attribute exposed by repr/tracebacks

• __repr__ returns only base_url and username; _password never echoed.

HIGH: PII leakage via response body in HTTPStatusError

• HTTPStatusError exceptions now surface only the status code and the high-level error.message field if present. Full body logged at DEBUG (operator-only) instead of bubbling into the LLM context.

MEDIUM: No jitter in retry backoff

• 429 retry sleep + RequestError retry sleep both add random.uniform(0, 1).

Already addressed in commit 831d343 (verified on this branch):

• 429 last_error tracking on max-retry exhaustion
• _parse_json helper for JSON tool args
• call_api lockdown to method+path allowlist
• OAuth/OIDC client_secret env-var indirection (secret never in tool args)

Acknowledged but not in this commit (separate follow-up):

• Refactor admin tools (register_oauth_application, register_oidc_provider, set_system_property, call_api) out of the LLM tool surface entirely and into explicit /connect skill scripts. Mitigated for now by the env-var indirection on secrets and the path-allowlist on call_api, but the right end state is to drop them from server.py altogether.

Repo hygiene:

• Added .gitignore for __pycache__/ and *.pyc in servicenow/.

Ready for re-review.

[saengland]

Follow-up commit 46ac8eb: admin tools off by default + table-API table denylist

You explicitly rejected the env-var-secret-indirection-only approach for the 3 admin tools in your review. Right end state is to extract them into the /connect skill (PR #6) as scripts the human runs explicitly. As an interim guard until that refactor lands:

Admin tools feature flag (off by default)

• New _ADMIN_TOOLS_ENABLED reads SERVICENOW_MCP_ENABLE_ADMIN_TOOLS env var (1/true/yes/on enables, anything else disables).
• New _require_admin_tools(tool_name) raises PermissionError with a clear message pointing at /connect and the env-var unblock procedure.
• Wired the guard into register_oauth_application, register_oidc_provider, set_system_property.
• Updated docstrings on each to label them ADMIN TOOL with the unblock procedure.
• LLM-driven prompt-injection invocations get a hard error with no side effect.

call_api data-permissive table allowlist (HIGH per your discussion_r3174960930)

The previous method+path allowlist blocked admin endpoint paths but permitted non-GET on /api/now/table/* unconditionally - so prompt injection could still:

DELETE /api/now/table/sys_user/<admin>
PATCH  /api/now/table/sys_properties/<security-prop>
POST   /api/now/table/sys_user_grmember          (privilege escalation)

Added _CALL_API_TABLE_DENYLIST_NON_GET covering sys_user, sys_user_grmember, sys_user_group, sys_user_role, sys_user_has_role, sys_properties, sys_audit, sys_audit_delete, sys_security_acl, sys_security_diag, oauth_entity, oauth_entity_profile, sys_oidc_provider, sys_certificate, sys_script*, sys_ws_operation. New _CALL_API_TABLE_PATH_RE parses the table name out of the path; on non-GET methods, denied tables raise PermissionError. GET (read) still permitted because resolve_user etc. need to query sys_user.

Audit of remaining items from your PR #15 review

Comment	Status
Encoded-query injection (9 sites, 20 fields)	✅ in 5df862b (_q() helper)
Path traversal table/sys_id regex validation	✅ in 5df862b
Mutating-CRUD denylist on protected tables	✅ in 5df862b
HTTPS enforcement on instance URL	✅ in 5df862b
Domain allowlist (*.service-now.com)	⚠️ not done - HTTPS check landed but domain check missed. Open follow-up.
__repr__ hides password	✅ in 5df862b
follow_redirects=False	✅ in 5df862b
Explicit verify=True	⚠️ not done - relies on httpx default. Open follow-up.
Reuse AsyncClient across calls	⚠️ not done - still per-call. Open follow-up.
httpx logger silencing	✅ in 5df862b
ServiceNow error PII strip	✅ in 5df862b
Retry jitter	✅ in 5df862b
delete_record denylist	✅ in 5df862b (denylist applies to mutating CRUD on client)
Admin tools out of LLM surface	✅ this commit (gated; full extraction = separate PR)
call_api table denylist on non-GET	✅ this commit
_fmt per-tool field allowlist	⚠️ not done - MEDIUM. Open follow-up.

Open follow-ups (separate PR after merge)

1. Full extraction of admin tools into solutions/ess-maker-skills/scripts/connect_servicenow.py invoked from /connect skill, removing them from server.py entirely
2. *.service-now.com domain allowlist with SERVICENOW_ALLOWED_HOSTS override
3. Explicit verify=True + reuse single AsyncClient across calls (move to __init__, add aclose())
4. _fmt field allowlist per tool

Ready for re-review.

[saengland]

This PR was prematurely merged earlier today during a tooling incident. main and main2 have been rolled back; replacement PR with the same branch and code is now open as #31 for proper review.