Fix API URL, auth loop, credential sanitization, and expand MCP toolset#3
Open
SecOps-7 wants to merge 62 commits into
Open
Fix API URL, auth loop, credential sanitization, and expand MCP toolset#3SecOps-7 wants to merge 62 commits into
SecOps-7 wants to merge 62 commits into
Conversation
Security:
- Remove hardcoded credentials from CLAUDE.md, sdp-oauth-client.cjs
- Remove customer-specific fallback values (portal, domain, instance) from
all source files; configuration now entirely env-var driven
- Remove hardcoded server IP and local paths from working-sse-server.cjs
Runtime:
- Bump Node.js engine requirement and @types/node to >=22
Bug fixes:
- Fix closure_code sent as plain string instead of { name } object (400 error)
- Fix resolution field format — now always sent as { content: "..." }
- Fix start_index pagination (was incorrectly set to 1 for first page)
- Fix priority name regression — restore 'z - Medium' (tenant-specific name)
New features:
- Add deleteRequest and addAttachment to API client and as MCP tools
- Add shared PRIORITY_NAMES constant; remove 3 duplicated inline priority maps
- Re-enable requester field and priority on createRequest
- Remove hardcoded subcategory default ('Not in list') from createRequest
- Wire list_technicians, get_technician, find_technician to real SDPUsersAPI
- Expand create_request and update_request MCP tool schemas with missing fields
- Add advanced_search_requests MCP tool with structured criteria support
- getStatuses() now tries API first and falls back to hardcoded list
Docs:
- Add CHANGELOG.md documenting all session changes
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- close_request now uses POST /requests/{id}/close (dedicated endpoint)
and only accepts closure_comments (max 250, auto-truncated) + closure_code
- Add closure_resolution tool: sets resolution.content via PUT /requests/{id}
with no character limit — for full documented solutions
- Add update_status tool: sets status only via PUT /requests/{id}
(e.g. On Hold, In Progress, Open) without triggering a close
- Add setResolution() and updateStatus() methods to SDPAPIClientV2
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add require('dotenv').config() as first line of working-sse-server.cjs
so .env file is loaded before any other module initialises
- Explicitly pass SDP_BASE_URL (as customDomain) and SDP_REFRESH_TOKEN
to SDPAPIClientV2 constructor instead of relying on indirect env fallbacks
- Fix .env.example variable names to match what the code actually reads:
SDP_OAUTH_CLIENT_ID -> SDP_CLIENT_ID
SDP_OAUTH_CLIENT_SECRET -> SDP_CLIENT_SECRET
SDP_OAUTH_REFRESH_TOKEN -> SDP_REFRESH_TOKEN
- Remove hardcoded customer values from .env.example (domain, instance, portal)
- Add SERVER_HOST variable to .env.example
- Replace real-looking Redis password placeholder with generic one
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Resolution / close fixes:
- close_request now accepts optional resolution parameter and sends it
as part of POST /close — the only way to record resolution atomically
since SDP blocks PUT updates on closed requests (403/4002)
- closure_resolution now returns a clear error when called on an already-
closed ticket instead of a raw UNAUTHORISED message
- close_request and closure_resolution schema descriptions updated to make
the ordering constraint explicit for the AI model
Startup connection test:
- Add testConnection() to SDPAPIClientV2: calls GET /priorities (1 row)
to verify OAuth and reachability, returns instance name, URL, region
- working-sse-server.cjs runs testConnection() after listen() and logs
success (instance/URL/region) or failure (error + env var checklist)
Environment:
- Add require('dotenv').config() as first line of server entry point
- Pass SDP_BASE_URL and SDP_REFRESH_TOKEN explicitly to SDPAPIClientV2
- Fix .env.example variable names to match what the code reads
- Remove hardcoded customer values from .env.example
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…est" This reverts commit 0f551fb.
- Add testConnection() to SDPAPIClientV2: calls GET /priorities (1 row) to verify OAuth and reachability, returns instance, URL, region - Log success or failure at startup with actionable env var checklist Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SDP_INSTANCE_NAME was not being passed to the SDPAPIClientV2 constructor, so this.instanceName resolved to undefined and every API URL became https://helpdesk.pttg.com/app/undefined/api/v3. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Both properties read from SDP_PORTAL_NAME — having two was redundant. Remove this.instanceName everywhere and use this.portalName throughout. Remove SDP_INSTANCE_NAME from .env.example. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace real client IDs, secrets, and tokens across all config files, scripts, and docs with YOUR_CLIENT_ID / YOUR_CLIENT_SECRET / YOUR_REFRESH_TOKEN placeholders. Remove .sdp-tokens.json from git tracking — it is already in .gitignore and should never have been committed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ic log - Restore this.instanceName (reads SDP_PORTAL_NAME) in API client and metadata client so the URL path matches the original repo's pattern - Use instanceName (not portalName) in all baseURL construction - Remove customDomain and refreshToken from working-sse-server.cjs constructor call — let the client read SDP_BASE_URL from env directly (matches original) - Add startup diagnostic log printing SDP_BASE_URL, SDP_PORTAL_NAME, and whether credentials are set, to surface misconfiguration immediately Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add _retry flag to cap 401 token-refresh retries at one attempt per request — prevents hammering the token endpoint when the URL or portal name is wrong and auth keeps failing - Log the actual 401 response body so the real rejection reason is visible - Log the constructed API baseURL at startup to confirm the full URL Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SDP instance uses https://sdpondemand.manageengine.com/api/v3/... not /app/{name}/api/v3/... — remove the portal name path segment from baseURL construction in both the API client and metadata client. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GET /priorities returns HTML 401 for this SDP instance — it is not a valid test endpoint. Confirm connectivity by successfully obtaining an access token instead of making an API call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Correct environment variable names (SDP_CLIENT_ID/SECRET/REFRESH_TOKEN)
- Remove deprecated SDP_INSTANCE_NAME, unify into SDP_PORTAL_NAME
- Document API URL fix (no /app/{name} segment)
- Add 401 retry loop fix and testConnection notes
- Add Azure deployment troubleshooting
- Add SDP-MCP_Usage.md usage guide
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Implements the MCP resources protocol to surface SDP-MCP_Usage.md
content as structured context for Copilot Studio agents:
- Define SDP_RESOURCES with 6 named resources:
sdp://usage/api-rules, field-formats, tool-reference,
error-codes, action-sequences, decision-matrix
- Declare resources: {} in initialize capabilities
- Implement resources/list handler (returns all 6 resources)
- Implement resources/read handler (returns content by URI)
- Add get_usage_guide tool — returns any resource section as
both a text content item and a typed resource content item,
satisfying the Copilot Studio "resource as tool output" requirement
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents API URL fix, 401 retry loop fix, testConnection fix, env var unification, Azure deployment support, SDP-MCP_Usage.md, README updates, MCP Resources implementation, and get_usage_guide tool. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…r health - Extract tool implementations into src/tools/requests.cjs (16 tools), src/tools/technicians.cjs (3 tools), src/tools/metadata.cjs (3 tools) - Slim working-sse-server.cjs from 1769 → 246 lines using factory pattern - Add src/mcp-prompts.cjs with 4 prompt templates (resolve, triage, escalate, follow_up) wired to prompts/list and prompts/get protocol handlers - Add src/mcp-resources.cjs with 6 usage guide resources; server now uses listResources/readResource from this module instead of inline constant - Add input validation: subject ≤250 chars, impact_details ≤250 chars, closure_code against known enum at tool layer - Upgrade /health endpoint to async with OAuth token probe, auth_status, instance, base_url, and data_center fields - Remove dead code: sdp-api-client.cjs, sdp-api-client-enhanced.cjs, simple-sse-server.cjs, index-sse-simple.ts, index.ts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
get_request now accepts either the short display_id (e.g. "31230") or the full 17-digit internal ID. IDs of 10 digits or fewer are treated as display_ids and resolved to the internal ID via advanced_search_requests before fetching full details. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Four issues corrected against the SDP v3 API documentation: 1. Single-element criteria array now unwrapped to a plain object — the API expects an object for single criteria, not a one-element array. 2. logical_operator stripped from the first element of multi-criteria arrays — the API rejects queries where the first criterion carries logical_operator. 3. Replaced page parameter with start_index ((page-1)*rowCount) — aligns with listRequests and the primary documented pagination parameter. 4. display_id lookup in get_request now passes value as integer (parseInt) — display_id is type long in the API; sending a string caused type mismatch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Success path of the response interceptor now logs total_count, has_more, and start_index for all list responses. When a search returns zero results, the search_criteria that was sent is also logged so the mismatch can be identified from server logs without needing a debugger. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot Studio's AI cannot construct the structured criteria array required by advanced_search_requests from natural language queries. Adding these two common person-based filters as simple string params on list_requests lets the AI call the tool without user prompting. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- list_requests: rewrote description with explicit examples to make it the obvious choice for common queries (by technician, requester, status, priority); added 'in progress' and 'on hold' to status enum - advanced_search_requests: description now says "only use when list_requests filters are not sufficient"; fixed field example from requester.name to requester.email_id - close_request: removed internal API path from description - add_note: clarified portal visibility vs email; points to reply_to_requester - search_requests: clarified it is full-text keyword search Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
GET /requests rejects an array of search_criteria for AND logic. The correct format is to put additional criteria as children of the first criterion with logical_operator: AND. The flat array format only works for OR conditions between the same field. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reverts the technician_email/requester_email additions to list_requests and the listRequests API client method. The search_criteria changes caused 400 errors from the SDP API when combining multiple filters. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Switch back to PUT /requests/{id} from POST /requests/{id}/close;
the /close endpoint returns "Invalid Method" on this SDP instance
- Accept resolution (not closure_comments) to match what the
close_request tool passes; resolution text is now correctly sent
- Send resolution as both closure_info.closure_comments (truncated
to 250 chars) and resolution.content (full text)
- Retain status: { name: "Closed" } to trigger the state transition
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PUT /requests/{id} rejects closure_code in closure_info with a 4001
validation error. Map the closure_code value to the appropriate status
name instead: Resolved→Resolved, Cancelled→Cancelled, Closed/Duplicate→Closed.
The closure_code parameter is still accepted by the tool so the AI can
express intent; it just drives the status field rather than being sent
directly to the API.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- response.data.request_notes was always undefined; correct key is notes - Add input_data with list_info (row_count: 100, sort asc by created_time) so the API returns all notes rather than a default-sized page Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds 'Tool: <name>' line immediately after 'Received request: tools/call' so the specific tool being invoked is clear without digging into payloads. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
closure_comments rejects HTML — the AI was wrapping resolution text in <p> tags which caused a 4001 validation error from SDP. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This reverts commit 83997c7.
closure_info.closure_comments is plain text only — SDP rejects HTML with a 4001 validation error. Strip tags and decode common HTML entities before sending. resolution.content supports HTML so it is left as-is. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
list_requests does not support technician/requester email filtering — those were reverted. Update description to accurately reflect that advanced_search_requests is the right tool for those filters. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…estions Add field enum, condition enum, and natural-language→criteria mapping in the description so the AI can construct criteria arrays from context without asking the user for field paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…icker prompt Copilot Studio renders enum fields as user-facing pickers instead of letting the AI infer values from context. Move valid values into description text only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replaces the criteria array (unusable from Copilot Studio) with flat params matching the list_requests pattern: technician_email, requester_email, subject_contains, category, group, created_after, created_before, due_before, status, priority. A new searchRequestsFlat() method in the API client builds search_criteria (single object or children nesting) and filter_by internally. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…for status Children nesting and arrays both cause 400 on this SDP instance. Status/priority always go via filter_by; advanced filters use a single plain-object search_criteria. Multiple advanced filters warn and use only the first. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
filter_by and search_criteria do not combine on this instance — filter_by wins and search_criteria is silently ignored. When an advanced filter is present, omit filter_by and apply status/priority filtering in JavaScript on the results. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Instead of search_criteria (which conflicts with filter_by), pass technician, requester, status, priority, category, group directly as reference objects in list_info. Falls back to search_criteria only for subject/date filters. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The SDP API requires "and"/"or" (lowercase) not "AND"/"OR" — uppercase was causing Tomcat 400 rejections. Restores array format for multi-criteria search in searchRequestsFlat and normalises case in advancedSearchRequests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removed stale partial block that was left in searchRequestsFlat() during the previous rewrite, causing a SyntaxError on module load in Node. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Switch multi-criteria search from flat array to the children object format required by this SDP Cloud instance. Also stringify row_count/start_index and use 1-based start_index to match the documented API expectations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Status values in children criteria must be lowercase (e.g. "open" not "Open") to match the format accepted by this SDP Cloud instance. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SDP Cloud requires Content-Type: application/x-www-form-urlencoded on all requests including GET. Without it, multi-criteria search_criteria calls return a Tomcat 400 Bad Request. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add custom paramsSerializer using encodeURIComponent to ensure square brackets and other special characters in JSON are properly percent-encoded (fixes Tomcat 400 caused by literal [ ] in query string) - Log the full encoded URL for every request with input_data so the exact query string sent to the API is visible in logs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Ensures technician and requester email values are always lowercased before being sent in the API search criteria. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- listRequests multi-filter: convert flat array to children nested format (flat array causes Tomcat 400); lowercase status values in criteria - advancedSearchRequests: same children format fix for multi-element arrays; row_count/start_index now strings with 1-based start_index - searchRequests: row_count/start_index as strings, 1-based start_index - getRequestConversation: row_count as string - createRequest/updateRequest: lowercase technician_email before sending - searchRequestsByRequesterEmail/TechnicianEmail: lowercase email values Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Close SSE connection 60s before Easy Auth token expiry so the client reconnects and receives a fresh token before the session goes stale - Fall back to 55-minute cycle when X-MS-CLIENT-PRINCIPAL header is absent - Log APPID and NAME claims on every new connection for identity visibility - Send retry: 0 before closing to prompt immediate client reconnection Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sy Auth" This reverts commit 4e9dad4.
- create_request: remove hard-coded defaults (mode, request_type, urgency, level, impact, category, status, priority); only send explicitly provided fields and let SDP apply its own instance defaults - create_request: lowercase requester_email before sending - update_status: add optional status_change_comments field required by SDP when setting On Hold; thread through tool schema, implementation, and client - get_metadata / sdp-api-metadata: add Content-Type and paramsSerializer to axios instance to match main client; fixes 401s on priorities, categories, and templates endpoints - sdp-api-users: fix row_count/start_index to strings, start_index to 1-based, and search_criteria from flat array to plain object - requests tool: remove priority enum restriction; accept any string value Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- All request_id schema descriptions now explicitly require the full internal ID (e.g. 97837000038081358) and warn against using the short display ID - update_request schema: add status_change_comments field - updateStatus / updateRequest: route comments to onhold_scheduler.comments when status is On Hold; use status_change_comments for all other statuses - updateRequest: add status_change_comments handling (was missing entirely) - get_request: fix display_id search value from parseInt (number) to string — SDP criteria values must be strings; parseInt caused 0-result lookups - mcp-resources api-rules: document internal ID requirement, fix start_index note from 0-based to 1-based, add On Hold/Cancelled comment requirement - mcp-resources field-formats: add REQUEST ID section with examples - mcp-resources tool-reference: clarify request_id and On Hold notes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Restores parseInt(internalId, 10) — reverts the string-value change that broke display_id resolution for get_request. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- sdp-api-users: listTechnicians and getTechnician now call /technicians
and /technicians/{id}; findTechnician uses search_fields.email_id or
.name directly against /technicians; listUsers and getUser call
/requesters and /requesters/{id}; all search switched from
search_criteria to search_fields
- sdp-api-metadata: getPriorities now falls back to known priority names
on API error (matching getStatuses pattern); row_count and start_index
are now strings with 1-based index in all five fetch methods
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- sdp-api-client-v2: log first 6 + last 4 chars of token on every request so token identity can be confirmed across clients - sdp-api-metadata: log token preview on every metadata request and add response error interceptor to capture full SDP error body on 401/4xx responses Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fetch subcategories for all categories in parallel and nest them under their parent category in the getAllMetadata response. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR brings bug fixes, security improvements, and feature additions developed while deploying the server in an Azure Web App environment against an
SDPOnDemand instance without a custom domain.
Bug Fixes
API URL Construction — Critical
The original code constructed the API base URL as
{SDP_BASE_URL}/app/{instanceName}/api/v3. For SDPOnDemand instances without a custom domain, the/app/{name}path segment is absent. The correct URL is{SDP_BASE_URL}/api/v3. Bothsdp-api-client-v2.cjsandsdp-api-metadata.cjsupdated.Infinite 401 Retry Loop — Critical
The axios response interceptor retried requests by calling
this.client(originalRequest), re-entering the same interceptor. When a 401 was caused by awrong URL (not an expired token), this created an infinite refresh→retry→401 loop until Zoho rate-limited the token endpoint. Fixed by adding a
_retryflag so each request is retried at most once.
testConnection()Calling Invalid EndpointThe startup connection test called
GET /prioritieswhich does not exist on all SDP instances and returns a Tomcat HTML 401 page, triggering the retryloop on every startup. Now verifies connectivity by successfully obtaining an OAuth access token instead.
Resolution Failing on Closed Tickets (403)
Setting resolution via
PUT /requests/{id}fails with 403 on closed tickets. Separated into a dedicatedclosure_resolutiontool requiring an openticket, and changed
close_requestto usePOST /requests/{id}/close.closure_comments400 ErrorSDP enforces a 250-character limit on
closure_comments. Added automatic truncation with…suffix.Environment Variable Loading
Added
require('dotenv').config()toworking-sse-server.cjsso local.envfiles load before module initialization.Security
Hardcoded Credentials Removed
All real client IDs, secrets, and tokens previously committed have been replaced with
YOUR_CLIENT_ID/YOUR_CLIENT_SECRET/YOUR_REFRESH_TOKENplaceholders across all config files, docs, and scripts.
.sdp-tokens.jsonUntrackedRemoved from git tracking (
git rm --cached) so token cache files are never committed..env.exampleVariable Names CorrectedRenamed
SDP_OAUTH_CLIENT_ID→SDP_CLIENT_ID,SDP_OAUTH_CLIENT_SECRET→SDP_CLIENT_SECRET,SDP_OAUTH_REFRESH_TOKEN→SDP_REFRESH_TOKENto matchwhat the server actually reads.
New MCP Tools
closure_resolutionupdate_statusadvanced_search_requestsreply_to_requesteradd_private_notesend_first_responseget_request_conversationStartup Diagnostics
Added env var log at startup showing all critical config values (secrets masked) and the constructed API base URL, making misconfiguration immediately
visible:
SDP env check:
SDP_BASE_URL = https://sdpondemand.manageengine.com
SDP_CLIENT_ID = (set)
SDP_REFRESH_TOKEN= (set)
SDP API baseURL: https://sdpondemand.manageengine.com/api/v3
✅ SDP API connection successful — OAuth token obtained
Schema Expansions
create_requestandupdate_requestschemas expanded with:urgency,impact,impact_details,group,technician,support_level,due_by_time,item,assets,tags,template.Other
CHANGELOG.mdstart_indexpagination corrected to0(0-based per API docs)SDP_INSTANCE_NAMEremoved from.env.example(consolidated intoSDP_PORTAL_NAME)