Bump actions/setup-python from 5 to 6 - #6
Merged
Conversation
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 5 to 6. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v5...v6) --- updated-dependencies: - dependency-name: actions/setup-python dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Contributor
Author
LabelsThe following labels could not be found: Please fix the above issues or remove invalid values from |
saengland
added a commit
that referenced
this pull request
Apr 30, 2026
… contract PR #15 changed register_oauth_application and register_oidc_provider to accept client_secret_env_var (env var NAME) instead of client_secret (raw value), keeping secrets out of MCP logs and LLM context. Update the four skill docs that call these tools: - step2-certificate.md, step2-entra.md, step2-graph.md: add a pre-step setting SERVICENOW_OIDC_CLIENT_SECRET_NOT_USED='not-used' (OIDC verification with Entra ID does not actually consume a client secret, but the ServiceNow record requires the field to be non-empty); pass that env var name to register_oidc_provider. - step2-oauth2.md: add a pre-step writing the freshly-generated CLIENT_SECRET to env var SERVICENOW_OAUTH_CLIENT_SECRET; pass that env var name to register_oauth_application.
This was referenced May 1, 2026
saengland
added a commit
that referenced
this pull request
May 4, 2026
…API table denylist John's review explicitly rejected the env-var-secret-indirection-only approach for the 3 admin tools. The 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-tool feature flag (off by default): - Added _ADMIN_TOOLS_ENABLED reading 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. - 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. call_api: data-permissive table allowlist (HIGH per John's PR #15 discussion_r3174960930): - Method+path allowlist already blocked admin endpoint paths but permitted non-GET on /api/now/table/* unconditionally - so a prompt-injected LLM 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 / role / group / 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. - _CALL_API_TABLE_PATH_RE parses the table name out of the path; on non-GET methods, denied tables raise PermissionError. GET (read) is permitted because the typed tools (resolve_user, etc.) need to query sys_user records. Follow-up (separate PR after merge): full extraction of admin tools into solutions/ess-maker-skills/scripts/connect_servicenow.py invoked from the /connect skill, removing them from server.py entirely.
saengland
added a commit
that referenced
this pull request
May 4, 2026
…ols gated) * PR #5: ServiceNow MCP server 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 for this review: - 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) Reviewer: @GrahamMcMynn Tracker: #10 * PR #15: Address review - 429 last_error, json error handling, call_api lockdown, OAuth/OIDC secret env-var indirection * Rename folder solutions/ess-agent-kit -> solutions/ess-maker-skills rename solutions/ess-agent-kit -> solutions/ess-maker-skills * Address PR #15 review (johnguy0): encoded-query injection fix, path traversal validation, denylist server.py: - Added _q() helper that escapes ServiceNow encoded-query special characters (^, null, CR, LF). The encoded-query syntax uses ^ as AND and ^OR as OR; embedding either inside a value lets prompt-injection break out of a single filter clause and append additional ones (e.g. query="x^OR1=1" returns all records; query="x^ORassigned_to=admin" leaks records assigned to a different user). - Wired _q() through every parts.append site in: search_incidents (5 fields), search_hr_cases (4), browse_service_catalog (2), search_cmdb_items (2), resolve_user (4), search_interactions (3). Total: 20 unsafe interpolations closed. client.py: - HTTPS enforcement on SERVICENOW_INSTANCE_URL - reject http:// to keep basic-auth credentials out of cleartext. - Validate `table` and `sys_id` URL path components against strict regexes (_TABLE_NAME_RE = ^[a-z][a-z0-9_]{0,63}$; _SYS_ID_RE = ^[0-9a-fA-F]{32}$). Combined with the protected-table denylist below, prevents path traversal attacks like sys_id="abc/sys_user/admin-id" hitting a different endpoint. - Denylist of protected tables for mutating CRUD: 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 on these now raise ValueError. Read-only query_table/get_record/get_stats can still reach them (otherwise we couldn't look up users for resolve_user). - Silence httpx + httpcore loggers at WARNING (DEBUG would echo Authorization headers). - __repr__ override hides _password from tracebacks. - httpx clients now follow_redirects=False (302 cannot replay Authorization to attacker-controlled host). - Add jitter to retry backoff (random.uniform(0,1)) on both 429 and RequestError paths. - Strip response body from HTTPStatusError exceptions; log full body at DEBUG (operator-only) instead of bubbling potentially-PII content into the LLM context. 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 so secrets are never passed as tool args Repo hygiene: - Added .gitignore for __pycache__/ and *.pyc in servicenow/. Acknowledged but not in this commit (separate follow-ups): - 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. They are still exposed here as MCP tools but call_api is path-allowlisted and OAuth/OIDC use env-var secret indirection. Need a follow-up commit to fully extract. * Address PR #15 review (johnguy0): admin tools off by default + table-API table denylist John's review explicitly rejected the env-var-secret-indirection-only approach for the 3 admin tools. The 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-tool feature flag (off by default): - Added _ADMIN_TOOLS_ENABLED reading 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. - 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. call_api: data-permissive table allowlist (HIGH per John's PR #15 discussion_r3174960930): - Method+path allowlist already blocked admin endpoint paths but permitted non-GET on /api/now/table/* unconditionally - so a prompt-injected LLM 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 / role / group / 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. - _CALL_API_TABLE_PATH_RE parses the table name out of the path; on non-GET methods, denied tables raise PermissionError. GET (read) is permitted because the typed tools (resolve_user, etc.) need to query sys_user records. Follow-up (separate PR after merge): full extraction of admin tools into solutions/ess-maker-skills/scripts/connect_servicenow.py invoked from the /connect skill, removing them from server.py entirely.
saengland
added a commit
that referenced
this pull request
May 5, 2026
… PR #6) Inside-repo rename per the naming policy locked in chore PR. Single skill markdown file; no behavior change.
saengland
added a commit
that referenced
this pull request
May 6, 2026
…view on PR #34. Issue #50 filed for the 7th (discover.py JSON output suggestion - touches scripts/, out of slice). #1 BLOCKER - bound the Azure device-code retry loop - Add an attempt counter persisted to my/.azure-login-attempts.json (initialize on first A.4 entry, increment per failed attempt, delete on success). - A.4 stops issuing new device codes when attempts == 3 and shows a "talk to your tenant admin" message. - A.5 manual-fallback branch now routes back to A.5 to verify (NOT A.4, which would reissue a fresh device code and overwrite the manual sign-in). #2 BLOCKER - extend /connect resume routing - Add entra, certificate, and federated (graph connector) to the step 2 unchecked branch in src/skills/connect/step1.md. - Mirror the same routes in the step 3 unchecked branch. - Add federated to the "switching auth" Step 2 router as well. #3 BLOCKER - stop persisting CERT_PASSWORD - Both "Immediately save CERT_PFX_PATH and CERT_PASSWORD" blocks in step2-certificate.md now save only PFX path, CER path, and thumbprint. Password stays in session memory. - Section 2.11 config.json schema drops `certPassword`, adds `certCerPath` and `certThumbprint`. - step3-certificate.md re-prompts for the password via vscode_askQuestions on resumed sessions. - Generation-time message updated to tell the user "save in your password manager now - this kit does not persist it." #4 fix Skip semantics on the readiness step - step3-flightcheck.md Skip branch no longer marks step 5 as [x]. It leaves it unchecked and stops, so the next /setup re-offers it. Option label changed to "Skip - remind me later" to match. #5 banner / tasks.md row-count drift - Fresh-start banner in onboarding/SKILL.md, the post-discovery banner in step1b.md, the pre-MCP banner in step2.md, and the post-MCP banner in step2.md all add row 5 ("Readiness check (optional)") so what the user sees matches the persisted task list. #6 add an "I'm not sure" diagnostic in servicenow/step1.md - New section 1.1b runs only if the user picked "I'm not sure" in 1.1. Asks one follow-up: "Do you go through a Microsoft sign-in page when you sign in to ServiceNow?" Yes -> entra, No -> basic, Still not sure -> entra with a one-line "switch later from /connect" callout.
saengland
added a commit
that referenced
this pull request
May 7, 2026
* PR #5: ServiceNow MCP server 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 for this review: - 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) Reviewer: @GrahamMcMynn Tracker: #10 * PR #15: Address review - 429 last_error, json error handling, call_api lockdown, OAuth/OIDC secret env-var indirection * Rename folder solutions/ess-agent-kit -> solutions/ess-maker-skills rename solutions/ess-agent-kit -> solutions/ess-maker-skills * Address PR #15 review (johnguy0): encoded-query injection fix, path traversal validation, denylist server.py: - Added _q() helper that escapes ServiceNow encoded-query special characters (^, null, CR, LF). The encoded-query syntax uses ^ as AND and ^OR as OR; embedding either inside a value lets prompt-injection break out of a single filter clause and append additional ones (e.g. query="x^OR1=1" returns all records; query="x^ORassigned_to=admin" leaks records assigned to a different user). - Wired _q() through every parts.append site in: search_incidents (5 fields), search_hr_cases (4), browse_service_catalog (2), search_cmdb_items (2), resolve_user (4), search_interactions (3). Total: 20 unsafe interpolations closed. client.py: - HTTPS enforcement on SERVICENOW_INSTANCE_URL - reject http:// to keep basic-auth credentials out of cleartext. - Validate `table` and `sys_id` URL path components against strict regexes (_TABLE_NAME_RE = ^[a-z][a-z0-9_]{0,63}$; _SYS_ID_RE = ^[0-9a-fA-F]{32}$). Combined with the protected-table denylist below, prevents path traversal attacks like sys_id="abc/sys_user/admin-id" hitting a different endpoint. - Denylist of protected tables for mutating CRUD: 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 on these now raise ValueError. Read-only query_table/get_record/get_stats can still reach them (otherwise we couldn't look up users for resolve_user). - Silence httpx + httpcore loggers at WARNING (DEBUG would echo Authorization headers). - __repr__ override hides _password from tracebacks. - httpx clients now follow_redirects=False (302 cannot replay Authorization to attacker-controlled host). - Add jitter to retry backoff (random.uniform(0,1)) on both 429 and RequestError paths. - Strip response body from HTTPStatusError exceptions; log full body at DEBUG (operator-only) instead of bubbling potentially-PII content into the LLM context. 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 so secrets are never passed as tool args Repo hygiene: - Added .gitignore for __pycache__/ and *.pyc in servicenow/. Acknowledged but not in this commit (separate follow-ups): - 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. They are still exposed here as MCP tools but call_api is path-allowlisted and OAuth/OIDC use env-var secret indirection. Need a follow-up commit to fully extract. * Address PR #15 review (johnguy0): admin tools off by default + table-API table denylist John's review explicitly rejected the env-var-secret-indirection-only approach for the 3 admin tools. The 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-tool feature flag (off by default): - Added _ADMIN_TOOLS_ENABLED reading 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. - 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. call_api: data-permissive table allowlist (HIGH per John's PR #15 discussion_r3174960930): - Method+path allowlist already blocked admin endpoint paths but permitted non-GET on /api/now/table/* unconditionally - so a prompt-injected LLM 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 / role / group / 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. - _CALL_API_TABLE_PATH_RE parses the table name out of the path; on non-GET methods, denied tables raise PermissionError. GET (read) is permitted because the typed tools (resolve_user, etc.) need to query sys_user records. Follow-up (separate PR after merge): full extraction of admin tools into solutions/ess-maker-skills/scripts/connect_servicenow.py invoked from the /connect skill, removing them from server.py entirely. * Address ruff E402 lint finding (slice PR #5) src/mcp/servicenow/server.py:166 - E402: 'import re as _re' was placed after non-import statements (the EXCLUDED_TABLES set definition). Moved to the top with the other stdlib imports per PEP 8. The compiled regex constant _CALL_API_TABLE_PATH_RE stays where it is (late module-level definition is fine; only the import had to move). Mechanical, behavior-preserving. * Round-1 review: address johnguy0 architect/security/perf/prompt findings (slice PR #5) ARCHITECT * Promote client._request to public client.request; _request kept as backward-compat alias. call_api in server.py now routes through client.request (no longer reaches into a private method). * Consolidate the table denylist into client.EXCLUDED_TABLES and import from server.py. The two parallel lists were already drifting (sys_user_grmember, sys_user_has_role, sys_oidc_provider, sys_security_diag, sys_script* were only on the server side; sys_log*, sys_log_transaction, oauth_entity_profile, sys_certificate were only on the client side). Single source now; the server-side _CALL_API_TABLE_DENYLIST_NON_GET is the import alias. SECURITY * Encoded-query injection: list_oauth_applications:641, list_oidc_providers:721, check_plugin_active:808, get_system_properties:824 were interpolating the LLM-supplied value raw into sysparm_query. Patched all four to route through _q(). Sample: 'name^OR1=1' would have recreated the encoded-query injection that _q() exists to prevent. * log_copilot_summary at server.py:621: data.update(extra) let LLM-supplied additional_fields clobber the tool-controlled u_interaction / u_summary / u_resolution. Reversed merge order to data = {**extra, **data} so tool-controlled fields always win. Added isinstance(extra, dict) guard so a JSON array/scalar surfaces a clear error rather than crashing the merge. * client.py logger.debug body emission removed. The comment above the exception promised the response body would not surface (PII / internal field metadata). The debug line undermined that promise: any operator who flipped the global log level to DEBUG would have leaked the body. If an operator needs the body for triage they should reproduce with httpx logging enabled explicitly. PERFORMANCE * Long-lived httpx.AsyncClient cached on self via _ensure_client (lazy init, asyncio.Lock around the create), closed on aclose(). Saves the 50-200ms TLS handshake per call across the typical 5-50 calls in an LLM session. * Retry path extended to 502/503/504 with the same backoff used for 429 (one transient gateway error during a deploy or restart no longer fails on the first try). * _parse_retry_after handles RFC 9110 HTTP-date format, falls back to delta-seconds, falls back to None on parse failure (the call site then uses 2**attempt). Previously int(retry_after) would raise on a date value. PROMPT-ENGINEERING * FastMCP server-level instructions now include a 'prefer typed tools' routing line. This reaches the LLM before any individual tool docstring; highest-leverage place to put routing. * query_table, create_record, update_record, delete_record docstrings each carry a one-sentence routing line pointing the LLM at the typed alternative. * search_incidents:assigned_to docstring fixed: ServiceNow matches sys_id only on assigned_to, so 'display name or sys_id' silently returned zero results when a name was passed. Doc now tells the LLM to call resolve_user first if it only has a name. * log_copilot_summary:additional_fields docstring lists the supported u_* columns (u_channel, u_outcome, u_session_id, u_disposition), states the JSON-object requirement, and warns that unknown columns surface as ServiceNow validation errors. All edits behavior-preserving for the supported call paths; tightening on the abuse paths. Lint clean (ruff). * Drop accidentally-committed __pycache__ from prior commit The previous commit picked up scripts/__pycache__/ from a local syntax check (python -m py_compile). They never should have been tracked. The repo .gitignore already excludes __pycache__/ globally; these slipped in via 'git add -A'. Remove + future commits will respect .gitignore. * ci: trigger CI re-run with updated workflow from main2 (PR #47)
saengland
added a commit
that referenced
this pull request
May 7, 2026
* PR #6: Connect + Onboarding skills Adds the connection setup and onboarding skill content under solutions/ess-agent-kit/src/skills/: - connect/ - integration setup flows for Azure, ServiceNow, and Workday (21 files) - onboarding/ - guided first-run experience for /setup (6 files) These are markdown skill files consumed by GitHub Copilot prompts to walk customers through environment connection and credential setup. SFI focus: any references to internal Microsoft tenants, hardcoded URLs, or sample credentials should be generic placeholders. Reviewer: @CavillMason Tracker: #10 * PR #6: Update ServiceNow connect skills for new client_secret_env_var contract PR #15 changed register_oauth_application and register_oidc_provider to accept client_secret_env_var (env var NAME) instead of client_secret (raw value), keeping secrets out of MCP logs and LLM context. Update the four skill docs that call these tools: - step2-certificate.md, step2-entra.md, step2-graph.md: add a pre-step setting SERVICENOW_OIDC_CLIENT_SECRET_NOT_USED='not-used' (OIDC verification with Entra ID does not actually consume a client secret, but the ServiceNow record requires the field to be non-empty); pass that env var name to register_oidc_provider. - step2-oauth2.md: add a pre-step writing the freshly-generated CLIENT_SECRET to env var SERVICENOW_OAUTH_CLIENT_SECRET; pass that env var name to register_oauth_application. * Rename folder solutions/ess-agent-kit -> solutions/ess-maker-skills rename solutions/ess-agent-kit -> solutions/ess-maker-skills * Rebrand: 'ESS Copilot Kit' -> 'ESS Maker Kit' in connect skill (slice PR #6) Inside-repo rename per the naming policy locked in chore PR. Single skill markdown file; no behavior change. * Round-2 fixes addressing all 6 in-scope findings from John's first review on PR #34. Issue #50 filed for the 7th (discover.py JSON output suggestion - touches scripts/, out of slice). #1 BLOCKER - bound the Azure device-code retry loop - Add an attempt counter persisted to my/.azure-login-attempts.json (initialize on first A.4 entry, increment per failed attempt, delete on success). - A.4 stops issuing new device codes when attempts == 3 and shows a "talk to your tenant admin" message. - A.5 manual-fallback branch now routes back to A.5 to verify (NOT A.4, which would reissue a fresh device code and overwrite the manual sign-in). #2 BLOCKER - extend /connect resume routing - Add entra, certificate, and federated (graph connector) to the step 2 unchecked branch in src/skills/connect/step1.md. - Mirror the same routes in the step 3 unchecked branch. - Add federated to the "switching auth" Step 2 router as well. #3 BLOCKER - stop persisting CERT_PASSWORD - Both "Immediately save CERT_PFX_PATH and CERT_PASSWORD" blocks in step2-certificate.md now save only PFX path, CER path, and thumbprint. Password stays in session memory. - Section 2.11 config.json schema drops `certPassword`, adds `certCerPath` and `certThumbprint`. - step3-certificate.md re-prompts for the password via vscode_askQuestions on resumed sessions. - Generation-time message updated to tell the user "save in your password manager now - this kit does not persist it." #4 fix Skip semantics on the readiness step - step3-flightcheck.md Skip branch no longer marks step 5 as [x]. It leaves it unchecked and stops, so the next /setup re-offers it. Option label changed to "Skip - remind me later" to match. #5 banner / tasks.md row-count drift - Fresh-start banner in onboarding/SKILL.md, the post-discovery banner in step1b.md, the pre-MCP banner in step2.md, and the post-MCP banner in step2.md all add row 5 ("Readiness check (optional)") so what the user sees matches the persisted task list. #6 add an "I'm not sure" diagnostic in servicenow/step1.md - New section 1.1b runs only if the user picked "I'm not sure" in 1.1. Asks one follow-up: "Do you go through a Microsoft sign-in page when you sign in to ServiceNow?" Yes -> entra, No -> basic, Still not sure -> entra with a one-line "switch later from /connect" callout. * fix(connect/azure): A.5 manual fallback bumps attempts to cap (resolves #34 thread) John flagged that when the manual az login fallback also fails, we re-read attempts (still 2 from A.4) and re-enter the manual flow indefinitely - the counter never advances past A.4. Now A.5 writes attempts=3 BEFORE waiting on 'done'. On the next A.5 iteration, the 'three failed attempts' branch fires with the talk-to-admin message and clears the counter. Manual fallback gets exactly one chance.
saengland
added a commit
that referenced
this pull request
May 7, 2026
…viewer approval; restoring branch for proper review
saengland
added a commit
that referenced
this pull request
May 7, 2026
* PR #6: Connect + Onboarding skills Adds the connection setup and onboarding skill content under solutions/ess-agent-kit/src/skills/: - connect/ - integration setup flows for Azure, ServiceNow, and Workday (21 files) - onboarding/ - guided first-run experience for /setup (6 files) These are markdown skill files consumed by GitHub Copilot prompts to walk customers through environment connection and credential setup. SFI focus: any references to internal Microsoft tenants, hardcoded URLs, or sample credentials should be generic placeholders. Reviewer: @CavillMason Tracker: #10 * PR #6: Update ServiceNow connect skills for new client_secret_env_var contract PR #15 changed register_oauth_application and register_oidc_provider to accept client_secret_env_var (env var NAME) instead of client_secret (raw value), keeping secrets out of MCP logs and LLM context. Update the four skill docs that call these tools: - step2-certificate.md, step2-entra.md, step2-graph.md: add a pre-step setting SERVICENOW_OIDC_CLIENT_SECRET_NOT_USED='not-used' (OIDC verification with Entra ID does not actually consume a client secret, but the ServiceNow record requires the field to be non-empty); pass that env var name to register_oidc_provider. - step2-oauth2.md: add a pre-step writing the freshly-generated CLIENT_SECRET to env var SERVICENOW_OAUTH_CLIENT_SECRET; pass that env var name to register_oauth_application. * Rename folder solutions/ess-agent-kit -> solutions/ess-maker-skills rename solutions/ess-agent-kit -> solutions/ess-maker-skills * Rebrand: 'ESS Copilot Kit' -> 'ESS Maker Kit' in connect skill (slice PR #6) Inside-repo rename per the naming policy locked in chore PR. Single skill markdown file; no behavior change. * Round-2 fixes addressing all 6 in-scope findings from John's first review on PR #34. Issue #50 filed for the 7th (discover.py JSON output suggestion - touches scripts/, out of slice). #1 BLOCKER - bound the Azure device-code retry loop - Add an attempt counter persisted to my/.azure-login-attempts.json (initialize on first A.4 entry, increment per failed attempt, delete on success). - A.4 stops issuing new device codes when attempts == 3 and shows a "talk to your tenant admin" message. - A.5 manual-fallback branch now routes back to A.5 to verify (NOT A.4, which would reissue a fresh device code and overwrite the manual sign-in). #2 BLOCKER - extend /connect resume routing - Add entra, certificate, and federated (graph connector) to the step 2 unchecked branch in src/skills/connect/step1.md. - Mirror the same routes in the step 3 unchecked branch. - Add federated to the "switching auth" Step 2 router as well. #3 BLOCKER - stop persisting CERT_PASSWORD - Both "Immediately save CERT_PFX_PATH and CERT_PASSWORD" blocks in step2-certificate.md now save only PFX path, CER path, and thumbprint. Password stays in session memory. - Section 2.11 config.json schema drops `certPassword`, adds `certCerPath` and `certThumbprint`. - step3-certificate.md re-prompts for the password via vscode_askQuestions on resumed sessions. - Generation-time message updated to tell the user "save in your password manager now - this kit does not persist it." #4 fix Skip semantics on the readiness step - step3-flightcheck.md Skip branch no longer marks step 5 as [x]. It leaves it unchecked and stops, so the next /setup re-offers it. Option label changed to "Skip - remind me later" to match. #5 banner / tasks.md row-count drift - Fresh-start banner in onboarding/SKILL.md, the post-discovery banner in step1b.md, the pre-MCP banner in step2.md, and the post-MCP banner in step2.md all add row 5 ("Readiness check (optional)") so what the user sees matches the persisted task list. #6 add an "I'm not sure" diagnostic in servicenow/step1.md - New section 1.1b runs only if the user picked "I'm not sure" in 1.1. Asks one follow-up: "Do you go through a Microsoft sign-in page when you sign in to ServiceNow?" Yes -> entra, No -> basic, Still not sure -> entra with a one-line "switch later from /connect" callout. * fix(connect/azure): A.5 manual fallback bumps attempts to cap (resolves #34 thread) John flagged that when the manual az login fallback also fails, we re-read attempts (still 2 from A.4) and re-enter the manual flow indefinitely - the counter never advances past A.4. Now A.5 writes attempts=3 BEFORE waiting on 'done'. On the next A.5 iteration, the 'three failed attempts' branch fires with the talk-to-admin message and clears the counter. Manual fallback gets exactly one chance. * Round-2 review on PR #52: address all 6 of John's threads Two blockers: * (workday/step3.md:212) Drop hardcoded C:\Users\saengland\\...\\pwsh.exe path. Call 'pwsh' off PATH so it works for every contributor and on macOS/Linux. Dataverse MCP fallback already handles the not-installed case. * (workday/step2.md:412) Stop persisting isuWqlPassword/isuGenericPassword to my/connect/workday/config.json. Hold them in session memory only, mirroring the cert PFX rule one file over. ISU credentials are full reusable Workday user accounts and the Power Platform connection ref already stores them encrypted server-side after step 3. Four smaller fixes: * (servicenow/step2-certificate.md:123) Replace Guid.Substring(0,16) (~60 bits) with RandomNumberGenerator.GetBytes(24) Base64 (192 bits). Match the 2048-bit RSA key strength. * (workday/step3.md:309) Surface a Message block before the auto-push to the live agent so the user sees the consent + checkpoint name, not just the test result in 3.6. * (workday/step3.md:259) Drop the 'no, search for' edit-trace leftover. Just say 'search for **Environment variables**'. * (connect/step1.md:100) Qualify the cross-doc reference: 'src/skills/connect/servicenow/step1.md section 1.1' (the per-product file, not this top-level routing file).
saengland
added a commit
that referenced
this pull request
May 8, 2026
* RAI eval CSVs (4 files) - cleaned mojibake. The latin-1 / UTF-8 round-trip damage in StarterTestSets + TemplatedTestSets RAI-HR.csv and RAI-IT.csv (rows with 'they'll' / 'company-managed') is fixed. Files now pure ASCII; safety rows can pass CompareMeaning for the right reason. * ESSEvaluationSamples/README.md (#2) - added explicit warning that the safety sets ship with the OOB agent's exact refusal wording, with two recommended workarounds (replace the expected string per deployment, or swap CompareMeaning for a refusal-classifier judge). Customers no longer assume safety sets are tenant-agnostic when they aren't. * ESSEvaluationSamples/README.md (#3) - 'Example Rows' section now shows BOTH a concrete StarterTestSet row (PTO accrual policy answer, ready to upload) AND the templated <placeholder> rows under their respective labels. The internal contradiction with line 30's 'No changes needed' claim is resolved (line 30 now scopes to functional sets and explicitly excepts safety sets via the warning above). * ess-samples/README.md (#4) - dropped the 'sample is pending reorganization' deprecation banner. It contradicted the 'this is a vendored snapshot' notice 12 lines down. The vendored-snapshot framing IS the correct framing per Grounding Priority; banner was wrong, not the snapshot notice. * Facilities/EmployeeRegisterVehicle/topic.yaml (#5) - replaced silent 'default to USA' fallback with explicit fail-fast: a top-level ConditionGroup checks for blank Global.ESS_UserContext_Country_Code, sends 'I couldn't determine your region from your user profile' and CancelAllDialogs. No more inventing a default that masks user-context-load failures. * Facilities/EmployeeRegisterVehicle/topic.yaml (#6) - standardized Env.fac_ParkingServiceApiurl -> Env.fac_ParkingServiceApiUrl across both call sites (line 57 + the second HttpRequestAction). Now matches the casing of Env.fac_ParkingServiceApiOboScope.
saengland
added a commit
that referenced
this pull request
May 8, 2026
* PR #10: Sample topics + templates + test sets Adds sample content under solutions/ess-agent-kit/src/examples/ess-samples/: - ESSEvaluationSamples/ - starter and templated evaluation test sets (CSV) - Facilities/ - facilities scenarios (5 sample topics) - ServiceNow/ - HRSD, ITSM, Catalog scenarios (8 sample topics) - Workday/ - employee and manager scenarios (16 sample topics) 91 files total (YAML topic definitions, JSON workflow definitions, XML template config, CSV test sets). Sourced from Copilot Studio Samples repo - verify no customer-specific data slipped in. Reviewer: @CavillMason Tracker: #10 * Rename folder solutions/ess-agent-kit -> solutions/ess-maker-skills rename solutions/ess-agent-kit -> solutions/ess-maker-skills * Slice PR #10: append snapshot policy notice to ess-samples README Adds an 'ESS Maker Kit Snapshot Notice' section to the existing upstream README under src/examples/ess-samples/. Documents source URL, snapshot date (2026-04-29), refresh policy (hand-PR; sync_samples.py auto-fetcher was removed in slice PR #3), and the explicit 'do not edit these files - put customer scenarios under workspace/agents/{slug}/topics/ instead' rule. Mirrors the slice PR #9 reference-docs README. Addresses the 'feels wasteful to be cloning those' concern raised by @johnguy0; the duplication is the explicit tradeoff that gives the kit domain-correct first-turn output, and the snapshot stamp makes the staleness contract explicit. Also notes that the ServiceNow/ folder is kit-team-added (not in upstream microsoft/CopilotStudioSamples) for transparency. * Round-2 review on PR #56: address all 6 of John's threads * RAI eval CSVs (4 files) - cleaned mojibake. The latin-1 / UTF-8 round-trip damage in StarterTestSets + TemplatedTestSets RAI-HR.csv and RAI-IT.csv (rows with 'they'll' / 'company-managed') is fixed. Files now pure ASCII; safety rows can pass CompareMeaning for the right reason. * ESSEvaluationSamples/README.md (#2) - added explicit warning that the safety sets ship with the OOB agent's exact refusal wording, with two recommended workarounds (replace the expected string per deployment, or swap CompareMeaning for a refusal-classifier judge). Customers no longer assume safety sets are tenant-agnostic when they aren't. * ESSEvaluationSamples/README.md (#3) - 'Example Rows' section now shows BOTH a concrete StarterTestSet row (PTO accrual policy answer, ready to upload) AND the templated <placeholder> rows under their respective labels. The internal contradiction with line 30's 'No changes needed' claim is resolved (line 30 now scopes to functional sets and explicitly excepts safety sets via the warning above). * ess-samples/README.md (#4) - dropped the 'sample is pending reorganization' deprecation banner. It contradicted the 'this is a vendored snapshot' notice 12 lines down. The vendored-snapshot framing IS the correct framing per Grounding Priority; banner was wrong, not the snapshot notice. * Facilities/EmployeeRegisterVehicle/topic.yaml (#5) - replaced silent 'default to USA' fallback with explicit fail-fast: a top-level ConditionGroup checks for blank Global.ESS_UserContext_Country_Code, sends 'I couldn't determine your region from your user profile' and CancelAllDialogs. No more inventing a default that masks user-context-load failures. * Facilities/EmployeeRegisterVehicle/topic.yaml (#6) - standardized Env.fac_ParkingServiceApiurl -> Env.fac_ParkingServiceApiUrl across both call sites (line 57 + the second HttpRequestAction). Now matches the casing of Env.fac_ParkingServiceApiOboScope. * fix(eval-csvs): clean mojibake in 4 RAI CSVs (round-trip Win-1252/UTF-8 damage) PR #56 thread 1 follow-up. The earlier round-2 commit ran the cleanup script but byte-identical output meant git saw no diff. v3 of the script reverses the Win-1252->UTF-8 round-trip first, then maps any remaining curly punctuation (apostrophes, dashes, ellipsis, nbsp) to ASCII equivalents. All 4 RAI CSVs now contain zero non-ASCII bytes; safety rows can pass CompareMeaning for the right reason.
amilandi
pushed a commit
that referenced
this pull request
Jun 19, 2026
#4: settings.json comment preservation — Replace ConvertFrom-Json/ ConvertTo-Json with regex-based string manipulation that inserts or updates the essMaker.mode key without touching other content. #5: Settings backup/restore — applyChatOnlyLayout now saves original user global settings to globalState before overwriting. Restore Standard Layout reads from backup and restores original values (or removes the key if no original existed). #6: 5-second abort window before deleting non-git directories — both Windows and Mac installers now warn and give the user time to Ctrl+C before removing a directory that matches the repo name but isn't a git repository. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Dawn Jeong (daeunJe0ng)
added a commit
to daeunJe0ng/Employee-Self-Service-Agent-Developer-Kit
that referenced
this pull request
Aug 5, 2026
…I-granular exemptions, data-file scan) Addresses apurvabanka's review of the offline URL hygiene gate: - microsoft#2/microsoft#5 exemptions are now matched at URI granularity, not bare host. A dual-use host (www.workday.com is both a SAML/SOAP issuer identifier and a real fetchable domain) is exempt only in its issuer-identifier shape; a genuinely fetchable http://www.workday.com/<page> is now caught. Pure namespace hosts (schemas.xmlsoap.org, docs.oasis-open.org) are exempted by URI prefix. Removed the dead sts.windows.net entry (only ever appears as https in source). Added a test asserting every exemption corresponds to a real namespace/identifier http URI in source, so a dead or lazily added exemption fails the suite. - microsoft#3 the scan now covers data files (.yaml/.yml/.json) under the source dir, not just .py. Markdown docs stay excluded (not report-rendered). - microsoft#4 a templated host introduced over http (http://{...}) is now flagged as insecure. The "http:// + var" concatenation blind spot is documented explicitly. - microsoft#6 looks_like_host now rejects underscores and leading/trailing hyphens per DNS label rules. - #1 docstrings reworded to state honestly that the gate scans all source text (including docstrings/comments), is fail-closed on any insecure http URL, and does not isolate only report-emitted URLs (that would need AST dataflow, out of scope). Full offline flightcheck suite 1023 passed; ruff clean on both files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 34f86888-5007-409e-8ce7-29133ae9b26d
Dawn Jeong (daeunJe0ng)
added a commit
that referenced
this pull request
Aug 6, 2026
) * flightcheck: add offline URL host hygiene gate (catch typo'd/hallucinated link hosts before merge) FlightCheck emits portal, doc, and API URLs but nothing verifies the hosts are real and intended. A typo'd or hallucinated host (learn.microsft.com) or an http portal link sends the operator nowhere, and today that ships silently. Add a deterministic, offline test that scans FlightCheck source for every http(s) URL and asserts each static host is registered in a curated allowlist and that fetchable hosts use https. Wire it into ci.yml as a fast no-network job so it gates every PR. This is a host-level gate only; it does not verify a path is live (stale-path/404 detection is a separate networked concern that can reuse url_registry.py). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8fb19ec0-cad9-41c2-8064-2208b3844ede * flightcheck: rework URL hygiene gate to fail-closed https rule (drop self-referential host allowlist) The prior gate required every host in FlightCheck source to be registered in a curated allowlist. That was self-referential (the author who writes a typo also edits the allowlist), high-maintenance (each new legit host, even docstring-only ones, was a red build), and it flagged legitimate hosts such as schema.management.azure.com and www.microsoft.com that are not clickable-typo risks. Replace it with one durable, fail-closed rule: every fetchable URL must be https, unless its host is a known namespace/identifier URI (SOAP/SAML namespaces) that is legitimately http. New hosts are covered by default, no registry upkeep. Rename url_registry.py -> url_hygiene_rules.py to reflect the rule-based purpose; add a structural host-shape check. Broaden CI: replace the single-file url-hygiene job with a job that runs the whole offline FlightCheck pytest suite (1003 tests), which previously ran in CI at all. Link clickability is handled by the report renderer (PR #208); live-path/redirect checking is a separate networked concern kept out of this deterministic job. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 54c58357-b127-4686-a331-df0344225da8 * ci: make `pip install -e .[test]` work for the flightcheck-tests job (fix setuptools flat-layout discovery error) The new flightcheck-tests CI job installs the suite via `pip install -e .[test]`, which is also the command documented in pyproject.toml. On a clean runner this failed: setuptools flat-layout auto-discovery errors with "Multiple top-level packages discovered in a flat-layout: ['setup', 'samples', 'solutions']". This project is a test harness with no importable package of its own (code under test is put on sys.path via [tool.pytest.ini_options].pythonpath), so declare an explicit build backend and an empty module list. The editable install becomes a deps-only no-op, fixing the documented install command for CI and local developers alike. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 54c58357-b127-4686-a331-df0344225da8 * flightcheck: tighten url hygiene gate per PR #209 review (URI-granular exemptions, data-file scan) Addresses apurvabanka's review of the offline URL hygiene gate: - #2/#5 exemptions are now matched at URI granularity, not bare host. A dual-use host (www.workday.com is both a SAML/SOAP issuer identifier and a real fetchable domain) is exempt only in its issuer-identifier shape; a genuinely fetchable http://www.workday.com/<page> is now caught. Pure namespace hosts (schemas.xmlsoap.org, docs.oasis-open.org) are exempted by URI prefix. Removed the dead sts.windows.net entry (only ever appears as https in source). Added a test asserting every exemption corresponds to a real namespace/identifier http URI in source, so a dead or lazily added exemption fails the suite. - #3 the scan now covers data files (.yaml/.yml/.json) under the source dir, not just .py. Markdown docs stay excluded (not report-rendered). - #4 a templated host introduced over http (http://{...}) is now flagged as insecure. The "http:// + var" concatenation blind spot is documented explicitly. - #6 looks_like_host now rejects underscores and leading/trailing hyphens per DNS label rules. - #1 docstrings reworded to state honestly that the gate scans all source text (including docstrings/comments), is fail-closed on any insecure http URL, and does not isolate only report-emitted URLs (that would need AST dataflow, out of scope). Full offline flightcheck suite 1023 passed; ruff clean on both files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 34f86888-5007-409e-8ce7-29133ae9b26d * flightcheck: catch insecure templated-host URLs in url hygiene scan (match stated guarantee) The scan regex split the host at the first `}`, so a templated host with a real dotted suffix (`http://{org}.crm.dynamics.com/...`) captured only the dotless `{org}` token and was skipped. That contradicted the gate's stated guarantee that a templated host over http is caught, and let an insecure templated-subdomain deep-link (a real FlightCheck shape) slip through. Fix: host group now matches a run of literal domain chars and/or `{template}` segments, so a templated host with a dotted suffix is captured whole and evaluated, while bare placeholders (`host:port`, dotless `x`) still drop out. Extract per-text URL parsing into `_urls_in_text` and add a scan-level test proving the templated-host capture and the placeholder skip. Offline suite: 1044 passed; url hygiene 10 passed; ruff clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 34f86888-5007-409e-8ce7-29133ae9b26d --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8fb19ec0-cad9-41c2-8064-2208b3844ede Copilot-Session: 54c58357-b127-4686-a331-df0344225da8 Copilot-Session: 34f86888-5007-409e-8ce7-29133ae9b26d
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
1 participant
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.
Bumps actions/setup-python from 5 to 6.
Release notes
Sourced from actions/setup-python's releases.
... (truncated)
Commits
a309ff8Bump urllib3 from 2.6.0 to 2.6.3 in /tests/data (#1264)bfe8cc5Upgrade@actionsdependencies to Node 24 compatible versions (#1259)4f41a90Bump urllib3 from 2.5.0 to 2.6.0 in /tests/data (#1253)83679a8Bump@types/nodefrom 24.1.0 to 24.9.1 and update macos-13 to macos-15-intel ...bfc4944Bump prettier from 3.5.3 to 3.6.2 (#1234)97aeb3eBump requests from 2.32.2 to 2.32.4 in /tests/data (#1130)443da59Bump actions/publish-action from 0.3.0 to 0.4.0 & Documentation update for pi...cfd55cagraalpy: add graalpy early-access and windows builds (#880)bba65e5Bump typescript from 5.4.2 to 5.9.3 and update docs/advanced-usage.md (#1094)18566f8Improve wording and "fix example" (remove 3.13) on testing against pre-releas...Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)