You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit da75e4d
Browse filesBrowse the repository at this point in the historyBrowse files
Add URL validation for all ArmClient uses and add DNS->IP address resolution SSRF protections. (#3884)
* Add URL validation for all ArmClient uses and add DNS->IP address resolution SSRF protections. Introduce IEndpointValidator for improved unit testing and parallel test execution. Update documentation accordingly.
And a bunch of follow up squashed commits below...
Fix Fabric test compilation errors
Remove added difficulty around ServiceStartCommand testing.
Slight tinkering to HttpClientFactoryConfigurator.
Fix using sorting in NamespaceUpdateCommandTests.cs
Fix Speech live test regression caused by #3282.
Stabilize LoopbackHttpServer on macOS.
Application of IP address vs DefaultProxy is now calculated per-request due to dynamic nature of DefaultProxy, PACs, etc. Also other PR feedback.
More doc updates for using a proxy vs not and IP address filtering applied.
Remove NoSsrfClientName and disable auto-redirect for _all_ HttpClient instances of DI.
Enable NO_PROXY to hit the IP address filtering logic just like if a system/PAC proxy allows for bypassing a destination.
Clarify known limitation around a system-level dynamic proxies that change bypassing answers mid-request.
Fix networking unit tests by adding explicit proxy in DI. CI builds were picking up proxy setting from the host machine which prevented code paths related to our IP address filtering tests.
* Fix spelling error in FabricCoreSetupTests comment (rquests -> requests)
---------
Co-authored-by: Vinay Gera <vigera@microsoft.com>
Copy file name to clipboardExpand all lines: .github/skills/add-azure-mcp-tools/SKILL.md
+10-8Lines changed: 10 additions & 8 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -65,11 +65,12 @@ return $"Request failed: {requestFailedException.Message}"; // may include auth
65
65
66
66
### Safe Downstream Interactions
67
67
-**Never concatenate user input directly without prior validation** into URLs, shell commands, resource identifiers, query strings, etc.
68
-
- Use `EndpointValidator`from `Microsoft.Mcp.Core.Helpers` to guard all endpoint usage — choose the method that matches your scenario:
69
-
-**Azure service data-plane endpoint** (endpoint derived from a resource name, e.g. storage account, ACR, App Config): call `EndpointValidator.ValidateAzureServiceEndpoint(endpoint, serviceType, AzureService.CloudConfiguration.ArmEnvironment, executingToolNamespaceName)` before constructing the client. Pass the command's top-level tool namespace (which may differ from `serviceType`) so the namespace-scoped emergency override can be applied. This enforces the correct per-cloud domain suffix (e.g. `.blob.core.windows.net` / `.blob.core.chinacloudapi.cn`) and HTTPS.
68
+
- Use the existing `IAzureService` dependency or inject `IEndpointValidator`from `Microsoft.Mcp.Core.Helpers` to guard endpoint usage — choose the method that matches your scenario:
69
+
-**Azure service data-plane endpoint** (endpoint derived from a resource name, e.g. storage account, ACR, App Config): prefer `AzureService.ValidateAzureServiceEndpoint(endpoint: endpoint, serviceType: serviceType)` before constructing the client. Code without `IAzureService` may inject `IEndpointValidator` and call `ValidateAzureServiceEndpoint(endpoint, serviceType, armEnvironment)` with the configured Azure cloud. The validator obtains the currently executing tool namespace from `ICommandContextAccessor`; callers must not supply or infer it. This enforces the correct per-cloud domain suffix (e.g. `.blob.core.windows.net` / `.blob.core.chinacloudapi.cn`) and HTTPS.
70
70
-**User-supplied URL to a known external service** (e.g. a GitHub URL the user provides): call `EndpointValidator.ValidateExternalUrl(url, allowedHosts)` with an explicit allowlist of permitted hosts.
71
-
-**User-supplied target URL with no known domain** (e.g. a load-test target the user controls): call `EndpointValidator.ValidatePublicTargetUrl(url, logger, executingToolNamespaceName)`, which enforces HTTPS/HTTP-only schemes, rejects private/reserved IP ranges, rejects reserved hostnames, and resolves DNS to catch hostnames that map to internal IPs. Pass the command's top-level tool namespace so the namespace-scoped emergency override can be applied.
72
-
- For services that *construct* the endpoint internally (not from user input), use the cloud-type switch pattern (see [Phase 1c: Service Implementation](#1c-service-interface-and-implementation)) — `EndpointValidator` is not required in that case but `ValidateAzureServiceEndpoint` can be added as a defense-in-depth layer.
71
+
-**User-supplied target URL with no known domain** (e.g. a load-test target the user controls): call `AzureService.ValidatePublicTargetUrl(url: url)` or inject `IEndpointValidator` and call `ValidatePublicTargetUrl(url)`. This enforces HTTP(S)-only schemes, rejects private/reserved IP ranges and hostnames, and resolves DNS to catch hostnames that map to internal IPs. The validator applies namespace-scoped emergency overrides from the ambient command context.
72
+
- When a toolset directly injects `IEndpointValidator`, register it with `services.AddEndpointValidation()`. Reuse the host-owned validator and policy; do not construct a production validator or pass an executing namespace at tool boundaries.
73
+
- For services that construct endpoints internally, use the cloud-type switch pattern (see [Phase 1c: Service Implementation](#1c-service-interface-and-implementation)) and validate the completed endpoint before credential acquisition, client construction, or network use.
73
74
-**Control-plane operations** (ARM resource creation, RBAC assignments, policy etc.) do not need `EndpointValidator` because they go through the typed Azure SDK ARM client. Construct ARM resource IDs using `ResourceIdentifier` or collection helpers — never by string-interpolating subscription/resource-group/resource-name directly into a raw ARM path.
74
75
- For new commands and services, always pass `CancellationToken` as the final parameter to all async downstream calls and propagate it throughout — never substitute `CancellationToken.None` or `default` at call sites.
75
76
- Fail closed: if tenant, subscription, or resource context is ambiguous, return an explicit validation error and require the caller to specify the value. Do not silently pick a default.
@@ -97,10 +98,11 @@ Add tests proving that an invocation without security options sends or creates t
97
98
| Threat | Established mitigation in this project |
| Input abuse (oversized/malformed names) | Override `ValidateOptions` with resource-specific length and format checks using deterministic validation first (length bounds, allowed-value sets, character/category checks). For query inputs, use a dedicated validator class — see `CosmosQueryValidator.EnsureReadOnlySelect` (`tools/Azure.Mcp.Tools.Cosmos/src/Validation/CosmosQueryValidator.cs`) as a reference for length cap, keyword blocking, and injection pattern detection. |
100
-
| Injection into downstream systems | For user-supplied queries: use a validator class that enforces a single read-only statement, caps length, strips/blocks dangerous tokens, and detects tautology patterns. Do not interpolate user input into query strings directly — prefer parameterized APIs where available. For blob/resource URIs: call `EndpointValidator.ValidateAzureServiceEndpoint` before constructing any client (see `tools/Azure.Mcp.Tools.Compute/src/Services/ComputeService.cs` blob URI handling as a reference). |
101
+
| Injection into downstream systems | For user-supplied queries: use a validator class that enforces a single read-only statement, caps length, strips/blocks dangerous tokens, and detects tautology patterns. Do not interpolate user input into query strings directly — prefer parameterized APIs where available. For blob/resource URIs: call `IAzureService.ValidateAzureServiceEndpoint` or the injected `IEndpointValidator.ValidateAzureServiceEndpoint` before constructing any client (see `tools/Azure.Mcp.Tools.Compute/src/Services/ComputeService.cs` blob URI handling as a reference). |
101
102
| Secret leakage via logs or error responses | Log only individually named, non-sensitive fields: `options.Subscription`, `options.ResourceGroup`, `Name`. Never use `{@Options}` or log connection strings, keys, or endpoint values. Override `GetErrorMessage` to return actionable but non-revealing messages — strip raw `RequestFailedException` bodies that may contain tokens or account metadata. |
102
103
| Cross-tenant/resource confusion |`SubscriptionCommand` base class enforces that `--subscription` is always present and resolved via `ISubscriptionResolver` before `ExecuteAsync` is called. Pass `options.Tenant` to all service calls so `IAzureService` can validate tenant context per-request. Fail explicitly if tenant context is ambiguous — do not fall back silently. |
103
-
| SSRF-like endpoint misuse | Use `EndpointValidator` from `Microsoft.Mcp.Core.Helpers`: `ValidateAzureServiceEndpoint(endpoint, serviceType, armEnvironment, executingToolNamespaceName)` for Azure data-plane endpoints, `ValidateExternalUrl(url, allowedHosts)` for user-supplied URLs to known hosts, `ValidatePublicTargetUrl(url, logger, executingToolNamespaceName)` for arbitrary user-controlled targets (DNS-resolves and blocks private/reserved IPs). Pass the command's top-level tool namespace to namespace-aware overloads. |
104
+
| SSRF-like endpoint misuse | Prefer the existing `IAzureService` validation methods. Without that dependency, inject `IEndpointValidator`: call `ValidateAzureServiceEndpoint(endpoint, serviceType, armEnvironment)` for Azure data-plane endpoints and `ValidatePublicTargetUrl(url)` for arbitrary user-controlled targets. `EndpointValidator.ValidateExternalUrl(url, allowedHosts)` remains static for exact known external hosts. The injected validator resolves the command namespace through `ICommandContextAccessor`; callers never pass it. |
105
+
| Proxy behavior | Do not treat proxy routing as endpoint authorization. The shared factory selects one explicit proxy in `ALL_PROXY`, `HTTPS_PROXY`, then `HTTP_PROXY` order for both HTTP and HTTPS. Requests routed through a proxy omit transport DNS/IP checks because that proxy is the network boundary. `NO_PROXY`, operating-system bypass, and PAC `DIRECT` routes use the protected direct transport and retain filtering. Debug recording proxies remain a handler-wide exception. Endpoint validation remains required in every case. |
104
106
105
107
### Using AI to Generate Tool Code
106
108
@@ -111,7 +113,7 @@ Requirements:
111
113
- Validate user-controlled inputs in `ValidateOptions` using resource-specific rules (naming rules, allowed values, length caps) where applicable; do not use one generic rule for all options.
112
114
- Prefer SDK/runtime validators and deterministic checks for user input validation.
113
115
- Log only individually named, known-safe parameters; never log option objects, credentials, keys, connection strings, or other secret-bearing fields.
114
-
- For endpoint/URL inputs, use `EndpointValidator` methods appropriate to the scenario (`ValidateAzureServiceEndpoint`, `ValidateExternalUrl`, or `ValidatePublicTargetUrl`). Avoid direct interpolation of unvalidated input into URLs or downstream queries.
116
+
- For endpoint/URL inputs, prefer `IAzureService` validation methods or inject the host-owned `IEndpointValidator`; use static `EndpointValidator.ValidateExternalUrl` only for exact known external hosts. Avoid direct interpolation of unvalidated input into URLs or downstream queries.
115
117
- For create or create-or-update tools, explicitly configure secure resource defaults (private access, HTTPS and strong TLS, and Managed Identity/RBAC where supported). Any weaker behavior must require a narrowly named, documented opt-in option, and omission during an update must not weaken the resource.
116
118
- Add negative tests for relevant security cases introduced by the command (for example malformed names, invalid endpoint hosts, or unsafe query text) rather than a one-size-fits-all set of tests.
117
119
- Keep error messages actionable but non-revealing: avoid exposing stack traces, raw backend payloads, or sensitive values to callers.
@@ -1215,7 +1217,7 @@ Before creating the PR, verify all of these:
1215
1217
-[ ]`ValidateOptions` enforces format, length, and allowed-value constraints on all inputs — not only nullability
1216
1218
-[ ] No raw option objects logged — only individually named, known-safe parameters (e.g., `options.Subscription`, `Name`)
1217
1219
-[ ] No user input concatenated directly into URLs, resource identifiers, or command strings without prior allowlist validation
1218
-
-[ ] Data-plane endpoints validated with `EndpointValidator.ValidateAzureServiceEndpoint` (Azure services), `ValidateExternalUrl` (known external hosts), or `ValidatePublicTargetUrl` (arbitrary user-supplied targets) — never derived from raw user input without validation
1220
+
-[ ] Data-plane endpoints validated with `IAzureService` or injected `IEndpointValidator`; exact known external hosts validated with static `EndpointValidator.ValidateExternalUrl`; arbitrary public targets validated with `IAzureService.ValidatePublicTargetUrl` or `IEndpointValidator.ValidatePublicTargetUrl`
1219
1221
-[ ] Error messages are actionable but do not expose internal state, stack traces, or sensitive field values to callers
1220
1222
-[ ] Sensitive fields (keys, secrets, connection strings) are not returned in standard list/get responses unless the command is explicitly marked `Secret = true`
1221
1223
-[ ] Create and create-or-update requests explicitly set supported secure defaults: public access disabled, HTTPS and strong TLS required, and Managed Identity/RBAC preferred over local keys
0 commit comments