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
fix: share ARM request handling across clients, support HTTPS_PROXY and publish without Reader on the resource group - #336
Share one ARM HTTP client between the APIM client, the API Center client and the Key Vault pre-flight check, and fix the bugs that had built up in their separate copies. Also lets publish run without Reader on the resource group, makes the Key Vault check use the cloud selected with --cloud, and sends ARM requests through HTTPS_PROXY when it is set.
Why one shared client
ApimClient, ApicClient and keyvault-checker each had their own token cache, retry loop, error class, pagination and long-running-operation (LRO) polling — about 600 duplicated lines. The copies had drifted, and several of the differences were bugs:
API Center sent the bearer token to any URL in an Azure-AsyncOperation or Location header; APIM checked for HTTPS on a known ARM host. Neither checked nextLink.
API Center lost the status code on its final 429 and retried immediately on a non-numeric Retry-After; APIM had the same Retry-After problem on requests.
APIM kept polling a 404 for 7.5 minutes; API Center compared operation states case-sensitively.
The Key Vault check had no retry or 429 handling and always called the public cloud.
Fixing each copy separately would keep three implementations to maintain. src/clients/arm-http-client.ts holds the shared behaviour (tokens, retries, pagination, LRO polling, waiting for a deleted resource); each client keeps only its service-specific rules (for example APIM's 409 PessimisticConcurrencyConflict retry and wiki-500 handling). src/clients/preflight.ts holds the resource-group/service pre-flight check both clients use.
Changes
Publish pre-flight
A 403 on the resource-group read skips that check and the service check decides, for APIM and API Center. Reader on the resource group is no longer needed (docs updated). The generated identity guides and Copilot setup prompts now call it optional: with it, a missing resource group is reported clearly.
API Center's pre-flight messages use the APIM wording, and a 401 or 403 now gives an actionable message instead of the raw HTTP error. Its "Create it (Bicep/ARM)" hint becomes "Ensure the API Center service exists before publishing". APIM's messages are unchanged.
ARM requests (both clients and the Key Vault check)
Tokens come from @azure/identity's getBearerTokenProvider (the Azure SDK's token cycler) instead of a hand-written cache: one refresh is shared by concurrent callers, the token is refreshed in the background from 2 minutes before expiry (or the credential's refreshAfterTimestamp) while the current one is still used, and callers wait only within a second of expiry.
Polling and nextLink URLs are followed only over HTTPS on the client's own cloud's ARM host, so one cloud's token is never sent to another cloud (main's APIM client accepted the ARM hosts of all four clouds); a rejected polling header falls back to the next one.
Throttling waits follow the Azure SDK's throttlingRetryStrategy (@typespec/ts-http-runtime): on a 429 or 503, retry-after-ms, then x-ms-retry-after-ms, then Retry-After (seconds or an HTTP date) is waited for as sent, and 0 or a past date retries at once. A 429 with none of these headers readable waits 1, 2 and then 4 seconds, as on main (the SDK does not retry it); a 503 without one backs off as before. The final 429 keeps its status.
Unread response bodies (retried responses, ignored 404s, PUT/DELETE responses followed by polling) are released.
Requests to SAS links (export results) send no credentials: besides Authorization, API Center now also strips Proxy-Authorization and x-ms-authorization-auxiliary, as APIM did. A failed one names the URL without its query and keeps the body, with the SAS signature masked as *** in both the message and error.body (HTTP 403 from https://…blob.core.windows.net/api-export/<id>: <?xml…><Error><Code>AuthenticationFailed</Code>…), since the body is text the CLI doesn't control: a proxy's or firewall's page can quote or link to the URL. (The Azure Storage authentication errors seen in the integration run, "Signature did not match" and "Signature fields not well formed", don't quote it.) Any sig parameter is masked, also in a percent-encoded URL or after a JSON-escaped \u0026, its value running only over the characters a signature can have as written (base64, %XX escapes, a JSON \, URL-safe - and _); so is the request's own signature wherever else it appears, including JSON-escaped with \/ or \u002B, read as sent so an unencoded + stays a +. The rest of a firewall's block page or a storage error still shows. On main the whole body, signature included, went into the message.
API Center's retry messages use the shared format and are logged at debug inside a retry-tracking scope, as APIM's are.
A request the proxy refuses with 407 Proxy Authentication Required fails at once, without retries, saying to check the user name and password in the proxy URL (HTTPS_PROXY, ALL_PROXY or HTTP_PROXY). Only ARM requests are covered: @azure/identity reports a token request the proxy refuses as just network_error, with nothing to tell it apart.
A network error that outlasts the retries names its cause (fetch failed (getaddrinfo ENOTFOUND …)) instead of just fetch failed.
Long-running operations
A 404 while polling ends the poll (success for deletes) instead of timing out.
API Center polls a PUT that returns 200 with Azure-AsyncOperation or Operation-Location, and honours Operation-Location (after Azure-AsyncOperation); before, it ignored both.
After a PUT, APIM's provisioning-state poll fails at once on Canceled or Cancelled, as on Failed, and matches states ignoring case. A cancelled operation never succeeds; main kept polling for up to 60 seconds and then reported a timeout.
Azure-AsyncOperation / Operation-Location status bodies without a status keep polling; Location polls read provisioningState and fail on a failed operation-status body.
A Location poll that answers 2xx with a JSON body and no state is done: under ARM's asynchronous pattern a Location URL answers 202 while the operation runs and its result once it has finished. main's APIM client kept polling such a body until its 7.5-minute timeout.
A DELETE accepted without a polling URL waits until the resource is gone, whatever state it reports: APIM checks every 2 seconds for up to 60 seconds, as main's provisioning-state poll did, and API Center every 5 seconds for up to 10 minutes, its existing operation deadline. main stopped as soon as an APIM resource reported no provisioningState or Succeeded, and API Center returned at once.
Failed / Canceled / Cancelled are matched case-insensitively.
Key Vault pre-flight check
Uses the ARM endpoint and token scope of the cloud selected with --cloud.
Goes through the shared client (retries, token reuse per scope); a credential problem still warns and skips.
"Key Vault access confirmed" is logged only when access was confirmed.
Permission decisions are unchanged from main. The docs now list the ARM reads and the permission each needs, and state the known limitation that grants made through a group are not seen.
Proxy
ARM requests used Node's global fetch, which ignores HTTPS_PROXY / HTTP_PROXY on its own, although the troubleshooting docs told users behind a proxy to set HTTPS_PROXY.
At start-up the CLI now applies the Azure SDK's proxy rules (proxyPolicy, which @azure/identity uses for its token requests) to fetch through an undici global dispatcher, on every supported Node.js version: one proxy from HTTPS_PROXY, then ALL_PROXY, then HTTP_PROXY (upper case before lower case); NO_PROXY entries match an exact host or a leading-dot domain; proxy URL credentials are sent as Basic Proxy-Authorization.
The SDK has no public API for these rules, so they are reproduced; a side-by-side run against the SDK pipeline routed every tested environment identically. The proxy URL is never logged.
New dependency: undici ^7.30.1 (Node.js 20.18.1 and later).
User-visible changes
Error messages from failed requests read HttpError: HTTP <status>: [<code>] <message>: <target>: <detail>; …, with the details of the ARM error (nested ones included) in full, instead of HttpError: HTTP <status>: <full body>. A body with no ARM error is cut to its first 500 characters, the length main's API Center client used for operation failures. The full body stays on error.body. For example, an invalid policy now reads HTTP 400: [ValidationError] One or more fields contain incorrect values: representation: The 'set-header' start tag on line 4 position 6 does not match the end tag of 'inbound'. Line 5, position 5.
The error class keeps main's APIM name, HttpError, and is used for ARM calls and for the SAS links export actions return. ApicHttpError remains a deprecated alias, so imports and instanceof checks keep working, but API Center errors now print as HttpError instead of ApicHttpError.
Deletes accepted without a polling URL now wait for the resource to disappear instead of returning at once, and fail with "… was not deleted within 1m 0.0s …" (APIM) or "… within 10m 0.0s …" (API Center) if it doesn't.
With a wrong password in the proxy URL, ARM requests fail within a second with the 407 message above, instead of "fetch failed" after every retry.
Warnings and errors that report a caught error now add the messages of the errors it was caused by, in parentheses (for example fetch failed (getaddrinfo ENOTFOUND …)), wherever the CLI formats them with errorMessage, not only for a network error after the last retry.
The Key Vault check retries a 5xx for 7 to 9 seconds before warning (1, 2 and 4 seconds, plus up to 30% jitter).
With HTTPS_PROXY (or ALL_PROXY / HTTP_PROXY) set, ARM requests go through the proxy, chosen and bypassed as @azure/identity does.
Timings
All retry and polling intervals and deadlines are the values main already uses. The token refresh point is the SDK's (2 minutes before expiry, in the background) instead of main's 5 minutes, since token caching now uses getBearerTokenProvider. Server-requested waits are read as the Azure SDK reads them and are not bounded, as in the SDK. Between polls, a positive Retry-After sets the wait for every later poll until another replaces it, and a missing, unreadable, 0 or past value keeps the current wait. That is what main's APIM client and the SDK's LRO poller (@azure/core-lro) do; main's API Center client went back to 5 seconds when a poll had no header.
Tests
New tests/unit/lib/proxy.test.ts (includes a fetch through a local proxy), tests/unit/clients/apic-client.test.ts (API Center had no client tests), arm-http-client.test.ts, preflight.test.ts, error-message.test.ts.
Each behaviour fix has a test that fails without it.
Behaviour shared by both clients (tokens, retries, Retry-After, polling) is tested once, in arm-http-client.test.ts, which takes over main's APIM client tests of it; the client tests cover each client's own choices (URLs, statuses, intervals, deadlines, hosts, body release). The clients' pre-flight tests are consolidated into preflight.test.ts, keeping three per client for its own wording: a possibly wrong resource group, the permission error, and the missing service.
tests/unit/cli/index.test.ts runs the CLI to check the proxy is applied before a command runs.
npm test: all pass except the 2 environment-related package-build integration tests that also fail on main. npm run lint and npm run build are clean.
Verified against Azure
Integration runs on 2026-11-04 and 2026-11-05 against a StandardV2 APIM source and target, an API Center service and a Key Vault in the public cloud, with main as the baseline:
Unfiltered extract and publish, signed in with az login, and API Center extract and publish with a specification import: same output and target state as main.
Throttling and polling, with 429, 503 and polling headers injected through a local intercepting proxy: Retry-After in seconds, retry-after-ms before Retry-After, Retry-After as a date (future and past), 429 without a header (1, 2, 4 seconds, then the status), 503 with and without a wait.
Deletes with --delete-unmatched, pagination over 120 tags, and a nextLink rewritten to another host (the list fails and no token goes there).
The Key Vault check with direct access.
An invalid policy's error message, with APIM's details.
Proxy: HTTPS_PROXY, the right and a wrong password (fails at once with the 407 message, password never logged), NO_PROXY exact and domain matches, lower-case and ALL_PROXY / HTTP_PROXY variables, and NO_PROXY=*. The proxy being used, NO_PROXY exact matches and the wrong password ran on Node.js 22.0.0, 22.23.4 and 24.22.0; the other proxy cases on the default Node.js only.
A failed SAS download: an APIM specification export whose signature was altered in transit read HTTP 403 from https://…blob.core.windows.net/api-export/<id>: <?xml…><Error><Code>AuthenticationFailed</Code>…, with no query or signature in the log.
Covered by unit tests only: APIM's 60-second wait for a delete without a polling URL, nested error details, and these SAS masking cases: an unencoded + in the signature, a JSON-escaped separator, a sig value running only over the characters a signature can have, the JSON-escaped forms of the signature, and URLs percent-encoded twice or with sig first.
ARM requests use Node's global fetch, which ignores HTTPS_PROXY and
HTTP_PROXY on its own. The troubleshooting docs told users behind a
proxy to set HTTPS_PROXY, but requests still went direct.
Token requests from @azure/identity go through the Azure SDK's
proxyPolicy, which reads the variables itself. At start-up the CLI now
applies that policy's rules to fetch through an undici global
dispatcher, on every supported Node.js version:
- one proxy for every request: HTTPS_PROXY, then ALL_PROXY, then
HTTP_PROXY, each in upper case before lower case;
- NO_PROXY (then no_proxy) entries match an exact host, or a
leading-dot domain and its subdomains; other forms (*, ports, a bare
domain for its subdomains) are not supported, as in the SDK;
- a user name or password in the proxy URL is sent as Basic
Proxy-Authorization;
- HTTPS requests are tunnelled (CONNECT) and plain-HTTP requests are
forwarded to the proxy.
The SDK has no public API for these rules (core-rest-pipeline's
proxyPolicy is not meant for direct use), so they are reproduced here.
Run side by side, the SDK pipeline and the CLI picked the same proxy,
bypass and credentials for every environment tried. The proxy URL is
never logged.
Adds undici ^7.30.1, which supports Node.js 20.18.1 and later and sets
the global dispatcher that Node's built-in fetch uses.
…check
The Key Vault access check hard-coded https://management.azure.com for
both its token scope and every ARM request, so publishing Key Vault-backed
named values with --cloud china|usgov|germany queried the public cloud.
KeyVaultCheckContext now carries armBaseUrl and authScope. publish-command
passes the cloud resolved from --cloud in PublishConfig.cloud, and the
resource publisher hands its ARM endpoint and scope to the check, so it
targets the same cloud as the APIM client. Soft-skip and warning
behaviour is unchanged. Secret URI parsing already used only the first
host label and now has tests for sovereign vault suffixes.
validatePreFlight read the resource group before the APIM service, so a
principal scoped to the APIM service only got 403 on the resource group
read and publish failed before the service check ran.
A 403 on the resource group read is now logged at debug and the
resource group check is skipped; the APIM service check still runs and
decides. When the check was skipped and the service GET returns 404, the
error names the subscription and resource group and asks to verify them.
The 404 message for a missing resource group, 401 handling and all other
errors are unchanged.
Update the RBAC section of the CI/CD authentication guide: Reader on the
resource group is no longer required. The generated identity guides and
Copilot setup prompts say the same: Reader on the resource group is
optional, and with it a missing resource group is reported clearly.
FixesAzure#303
…p reads for API Center
ApicClient.validatePreFlight read the resource group the same way the
APIM client did before the previous commit, so `apiops apic publish`
still failed for a principal without Reader on the resource group.
Move the resource group and service checks into a shared
validateServicePreFlight helper in src/clients/preflight.ts, used by both
clients with their own wording. API Center now gets the same behaviour
as APIM: a 403 on the resource group read is logged at debug and the
service check decides, a skipped check widens the service 404 message,
and 401/403 errors become actionable messages instead of raw HTTP
errors. APIM messages are unchanged.
API Center messages now match the APIM wording and quoting; the
"Create it (Bicep/ARM)" hint is replaced by "Ensure the API Center
service exists before publishing".
Refs Azure#303
ApicClient had tests only for validatePreFlight. Add characterisation
tests for the rest of its ARM traffic to tests/unit/clients/apic-client.test.ts,
grouped by area, before the client is hardened and refactored:
- token: default and custom scope, caching across requests, one shared
in-flight token request, refresh within five minutes of expiry
- listResources: service, workspace and child collections with encoding,
nextLink pagination, 404 as an empty list, 403 as ApicHttpError
- getResource, putResource (200/201, 201 + Azure-AsyncOperation,
202 + Location), deleteResource (200/204/404, 202 + polling)
- exportSpecification (inline, link without ARM credentials, link retry,
404s, failures) and importSpecification (body, 202 + polling, failure)
- retry on 5xx and network errors, no retry on 4xx, 429 with numeric
Retry-After and without it
- LRO polling: in progress, Retry-After between polls, Failed/Canceled,
404 while polling a PUT, empty 200, ten-minute timeout
`fetch` is stubbed, @azure/identity is mocked and timers are faked.
Behaviours that are bugs (unvalidated LRO and nextLink hosts, the final
429, non-numeric Retry-After, case-sensitive LRO status, PUT 200 with
Azure-AsyncOperation, DELETE 202 without a polling header) are not
locked in here.
ApicClient's ARM handling lagged behind the APIM client:
- LRO URLs from Azure-AsyncOperation/Location were polled with the bearer
token without checking the host, and Operation-Location was ignored.
Polling URLs must now be HTTPS on a known ARM host (or a subdomain of
one), otherwise they are ignored with a warning. Operation-Location is
honoured after Azure-AsyncOperation.
- nextLink was followed with the token without checking the host. A
nextLink that is not HTTPS on a known ARM host now fails the list.
- The final 429 slept again and then threw a plain "Max retries exceeded"
error. It now throws ApicHttpError with status 429 and the ARM code.
- Retry-After was read with parseInt, so an HTTP-date gave delay(NaN) and
an immediate retry or poll. It is now parsed as delay-seconds or an
HTTP-date; anything else falls back to the back-off (requests) or the
default polling interval (LRO).
- LRO status was compared case-sensitively and "Cancelled" was not
terminal. Status is now compared case-insensitively.
- A PUT that returns 200 with Azure-AsyncOperation/Operation-Location was
not polled. It now is; a Location header on a 200 is still not polled.
- A DELETE that returns 202 without a (valid) polling header returned true
immediately. It now polls the resource until it is gone (404) or reports
a terminal provisioningState.
- skipAuth now also strips Proxy-Authorization and
x-ms-authorization-auxiliary, as the APIM client does.
The allowed ARM hosts come from a new getArmHosts() in cloud-config
instead of another hard-coded list.
ApimClient, ApicClient and the Key Vault pre-flight check each had their
own token cache, retry loop, error class, pagination and long-running
operation (LRO) polling. Move that into src/clients/arm-http-client.ts
(HttpError, ArmHttpClient, paginateArm, LRO polling, waitUntilGone) and
have all three use it. Each client keeps only its service-specific rules
(APIM's 409 PessimisticConcurrencyConflict retry, wiki 500 suppression,
DELETE 412 retries). Retry and polling intervals and deadlines are main's
values.
Requests:
- Tokens come from @azure/identity's getBearerTokenProvider (the Azure
SDK's token cycler) instead of a hand-written cache. The shared client
needs one token cache, and the SDK's already handles refreshing and
concurrent callers, so none is kept here: concurrent callers share one
refresh; from 2 minutes before expiry (or the credential's
refreshAfterTimestamp) the token is refreshed in the background while
the current one is used, instead of main's blocking refresh 5 minutes
before expiry.
- Polling and nextLink URLs are followed only over HTTPS on the client's
own cloud's ARM host, so one cloud's token is never sent to another
cloud's host (main's APIM client accepted the ARM hosts of all four
clouds); a rejected polling header falls back to the next.
- Throttling waits follow the Azure SDK's throttlingRetryStrategy: on a
429 or 503, retry-after-ms, then x-ms-retry-after-ms, then Retry-After
(seconds or an HTTP date) is waited for as sent; 0 or a past date
retries at once. A 429 without a readable header waits 1, 2 and then
4 seconds as main does; a 503 without one backs off. The final 429
keeps its status.
- Error messages carry the ARM code, message and the messages of its
details, nested ones included (APIM explains a ValidationError there),
in full, instead of the full response body, which stays on error.body.
A body with no ARM error is cut to its first 500 characters, the length
main's API Center client cut operation failures to. A failed
long-running operation's message has its details too.
- A failed download from a SAS link (an export's result) names the URL
without its query and keeps the body, with the SAS signature masked in
the message and in error.body. The body is text the CLI doesn't
control: a proxy's or firewall's page can quote or link to the URL.
(The Storage authentication errors seen in the integration run don't
quote it.) Any sig parameter is masked, also in a percent-encoded URL
or after a JSON-escaped separator (\u0026), its value running only
over the characters a signature can have as written (base64, %XX
escapes, a JSON '\', URL-safe '-' and '_'). So is the request's own
signature wherever else it appears, when it is one (the base64 of a
32-byte HMAC-SHA256), including JSON-escaped with \/ or \u002B. The
signature is read as sent, decoding only %XX escapes, so an unencoded
'+' stays a '+'. The rest of the body still shows. main put the whole
body, signature included, in the message.
- The error class is HttpError, the name the APIM client's had on main,
for ARM calls and the SAS links export actions return alike.
ApicHttpError remains as a deprecated alias, so API Center errors now
print as HttpError instead of ApicHttpError.
- API Center's retry messages use the shared format and go through
logRetry, so inside a retry-tracking scope they are logged at debug,
as APIM's are.
- Response bodies that are not read (retried responses, ignored 404s, PUT
and DELETE responses followed by polling, pre-flight responses) are
released so their connections are freed.
Long-running operations:
- Between polls a positive Retry-After is kept until another replaces
it (a missing, unreadable, 0 or past value keeps the current wait), as
main's APIM client and @azure/core-lro do; API Center used to go back
to its 5-second interval.
- A 404 while polling ends the poll (success for deletes) instead of
running to the timeout; states are matched case-insensitively.
- After a PUT, APIM's provisioning-state poll matches states ignoring
case and fails at once on Canceled or Cancelled, as on Failed: a
cancelled operation never succeeds, and main kept polling for up to
60 seconds and then reported a timeout.
- Azure-AsyncOperation and Operation-Location status bodies without a
status keep polling; a Location poll reads properties.provisioningState
and fails on an operation-status body reporting Failed, Canceled or
Cancelled; JSON null or non-object bodies no longer throw.
- A Location poll that answers 2xx with a JSON body and no state is
done: under ARM's asynchronous pattern a Location URL answers 202 while
the operation runs and its result once it has finished. main's APIM
client kept polling such a body until its 7.5-minute timeout.
- A DELETE accepted without a polling URL waits until the resource is
gone, whatever state it reports. APIM checks every 2 seconds for up to
60 seconds, main's limit; main stopped as soon as the resource reported
no provisioningState or Succeeded, so it usually reported success at
once. API Center checks every 5 seconds for up to 10 minutes, main's
operation deadline; the previous commit also stopped on a terminal
provisioningState, and it now checks presence only, without parsing
the body.
Key Vault check: uses the shared client (retries, one client per auth
scope); creates the client inside its soft-skip; logs "Key Vault access
confirmed" only when access was confirmed. Its access decisions are the
same as on main. Docs list the ARM reads it makes, the permission each
needs, when it fails a named value and its known limitation (grants
through a group are not seen).
The publish pre-flight helper uses HttpError and gets its own tests.
The clients accept an injected credential and wait function for tests.
Docs describe how 429 retries wait.
Client tests that only repeated the shared client's token, retry,
Retry-After and polling behaviour are removed, since
arm-http-client.test.ts covers it; each client keeps the tests for its
own choices (URLs, statuses, intervals, deadlines, hosts, body release).
The clients' pre-flight tests are consolidated into preflight.test.ts,
keeping three per client for its own wording: a possibly wrong resource
group, the permission error, and the missing service.
Non-OK checks that could never run (after requests that throw on those
statuses) are removed, and paginateArm's requestOptions can no longer
allow an error response to be read as a page.
With wrong credentials in HTTPS_PROXY, every ARM request was retried
three times and then failed with just "fetch failed": fetch puts the
reason (here the proxy's 407 Proxy Authentication Required) on the
error's cause, which no message showed.
A request the proxy refuses with 407, to a CONNECT tunnel or a forwarded
request, now fails at once, saying the proxy refused it and to check
the user name and password in the proxy URL. Other network errors are
still retried; when they outlast the retries, the error's message names
its cause ("fetch failed (getaddrinfo ENOTFOUND ...)"), and errorMessage
adds the causes of any error it is given. Both follow an error's causes
with one helper, errorCauses, which stops at a cause already seen so a
cycle of causes ends.
Only ARM requests are covered. A token request the proxy refuses fails in
@azure/identity with just "network_error", with no cause or status to
tell it from any other network failure.
Found by the ARM client integration tests.
This PR consolidates ARM request handling for APIM, API Center, and Key Vault pre-flight checks. It addresses #303 by allowing publish to continue when resource-group reads return 403, while retaining service-level validation.
Changes:
Shares authentication, retries, pagination, polling, response cleanup, and error formatting.
Adds environment-based proxy support and cloud-aware Key Vault checks.
Expands regression coverage and updates least-privilege authentication guidance.
File
Description
tests/unit/services/resource-publisher.test.ts
Tests cloud propagation to Key Vault checks.
tests/unit/services/publish-service.test.ts
Tests least-privilege publish and retry attribution.
tests/unit/services/keyvault-checker.test.ts
Covers shared-client behavior, clouds, and access diagnostics.
tests/unit/lib/proxy.test.ts
Tests proxy selection, bypass rules, and authentication.
tests/unit/lib/error-message.test.ts
Tests cause-chain formatting and cycle handling.
tests/unit/lib/cloud-config.test.ts
Tests cloud-specific ARM host selection.
tests/unit/clients/preflight.test.ts
Covers shared pre-flight checks and permission errors.
tests/unit/clients/apim-client.test.ts
Updates client tests and adds polling regressions.
The request's own signature was masked only in a fixed list of whole
forms (as sent, decoded, percent-encoded once or twice, and two
JSON-escaped variants). A page that wrote its + characters as HTML
character references (&Azure#43;, +), or escaped only some of them,
left the signature readable in the error message and in error.body.
The signature is now matched with each +, / and = in any of its forms
(as is, percent-encoded once or twice, JSON-escaped or as an HTML
character reference), in any mix.
PublishConfig.cloud is required, so that the Key Vault pre-flight check
always uses the cloud selected with --cloud, but the test fixtures that
build a PublishConfig left it out. Vitest doesn't type-check, so the
tests still ran; type-checking them reported 45 missing properties.
Each fixture now sets cloud to the public cloud.
The token was fetched once before the retry loop, so a long
server-requested wait could leave the next attempt with an expired
token. It is now asked for before every attempt; the provider returns
its cached token until refresh is due.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
A critical masking flaw leaves nearly the entire request signature exposed in error messages and HttpError.body when the echoed sig value contains HTML escapes.
The sig= parameter rule stops at an HTML character reference, so for a
link written as ?sig=A&Azure#43;... it masked only the first character. The
full-signature pattern then no longer matched, and the rest of the
signature stayed in the error message and HttpError.body. The request's
own signature is now masked first.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Polling bypasses APIM’s retry policy, causing a 409 PessimisticConcurrencyConflict to abort PUT/DELETE operations instead of retrying.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Polling bypasses APIM retry policy for 409 conflicts
src/clients/apim-client.ts:692
Calling the shared poller with this.http bypasses ApimClient.request, so polling GETs no longer receive APIM's isRetryableError predicate. A 409 PessimisticConcurrencyConflict during polling now aborts the PUT/DELETE immediately instead of being retried. Let the poller accept request options or a request callback, pass APIM's retry policy through, and add a regression covering a 409 poll followed by Succeeded.
APIM's retry of a 409 PessimisticConcurrencyConflict was merged into
each request by ApimClient, so the shared LRO poller, which calls the
ARM client directly, lost it: a 409 while polling failed the PUT or
DELETE at once, where main retried it. Each service now gives its
ArmHttpClient its request options once, and the client applies them to
every request, including pagination and polling; a request's own
options still override them.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The shared transport refactor affects authentication, proxy routing, and long-running operations across both services, but reported Azure runs omitted real asynchronous operations and token refresh across expiry, warranting maintainer validation.
No open issues 🎉
Review effort: Balanced
This branch has not been deployed
No deployments
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
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
Share one ARM HTTP client between the APIM client, the API Center client and the Key Vault pre-flight check, and fix the bugs that had built up in their separate copies. Also lets publish run without Reader on the resource group, makes the Key Vault check use the cloud selected with
--cloud, and sends ARM requests throughHTTPS_PROXYwhen it is set.Why one shared client
ApimClient,ApicClientandkeyvault-checkereach had their own token cache, retry loop, error class, pagination and long-running-operation (LRO) polling — about 600 duplicated lines. The copies had drifted, and several of the differences were bugs:Azure-AsyncOperationorLocationheader; APIM checked for HTTPS on a known ARM host. Neither checkednextLink.Retry-After; APIM had the sameRetry-Afterproblem on requests.Fixing each copy separately would keep three implementations to maintain.
src/clients/arm-http-client.tsholds the shared behaviour (tokens, retries, pagination, LRO polling, waiting for a deleted resource); each client keeps only its service-specific rules (for example APIM's 409PessimisticConcurrencyConflictretry and wiki-500 handling).src/clients/preflight.tsholds the resource-group/service pre-flight check both clients use.Changes
Publish pre-flight
ARM requests (both clients and the Key Vault check)
@azure/identity'sgetBearerTokenProvider(the Azure SDK's token cycler) instead of a hand-written cache: one refresh is shared by concurrent callers, the token is refreshed in the background from 2 minutes before expiry (or the credential'srefreshAfterTimestamp) while the current one is still used, and callers wait only within a second of expiry.nextLinkURLs are followed only over HTTPS on the client's own cloud's ARM host, so one cloud's token is never sent to another cloud (main's APIM client accepted the ARM hosts of all four clouds); a rejected polling header falls back to the next one.throttlingRetryStrategy(@typespec/ts-http-runtime): on a 429 or 503,retry-after-ms, thenx-ms-retry-after-ms, thenRetry-After(seconds or an HTTP date) is waited for as sent, and0or a past date retries at once. A 429 with none of these headers readable waits 1, 2 and then 4 seconds, as onmain(the SDK does not retry it); a 503 without one backs off as before. The final 429 keeps its status.Authorization, API Center now also stripsProxy-Authorizationandx-ms-authorization-auxiliary, as APIM did. A failed one names the URL without its query and keeps the body, with the SAS signature masked as***in both the message anderror.body(HTTP 403 from https://…blob.core.windows.net/api-export/<id>: <?xml…><Error><Code>AuthenticationFailed</Code>…), since the body is text the CLI doesn't control: a proxy's or firewall's page can quote or link to the URL. (The Azure Storage authentication errors seen in the integration run, "Signature did not match" and "Signature fields not well formed", don't quote it.) Anysigparameter is masked, also in a percent-encoded URL or after a JSON-escaped\u0026, its value running only over the characters a signature can have as written (base64,%XXescapes, a JSON\, URL-safe-and_); so is the request's own signature wherever else it appears, including JSON-escaped with\/or\u002B, read as sent so an unencoded+stays a+. The rest of a firewall's block page or a storage error still shows. Onmainthe whole body, signature included, went into the message.HTTPS_PROXY,ALL_PROXYorHTTP_PROXY). Only ARM requests are covered:@azure/identityreports a token request the proxy refuses as justnetwork_error, with nothing to tell it apart.fetch failed (getaddrinfo ENOTFOUND …)) instead of justfetch failed.Long-running operations
Azure-AsyncOperationorOperation-Location, and honoursOperation-Location(afterAzure-AsyncOperation); before, it ignored both.mainkept polling for up to 60 seconds and then reported a timeout.Azure-AsyncOperation/Operation-Locationstatus bodies without a status keep polling;Locationpolls readprovisioningStateand fail on a failed operation-status body.Locationpoll that answers 2xx with a JSON body and no state is done: under ARM's asynchronous pattern aLocationURL answers 202 while the operation runs and its result once it has finished.main's APIM client kept polling such a body until its 7.5-minute timeout.main's provisioning-state poll did, and API Center every 5 seconds for up to 10 minutes, its existing operation deadline.mainstopped as soon as an APIM resource reported noprovisioningStateor Succeeded, and API Center returned at once.Key Vault pre-flight check
--cloud.main. The docs now list the ARM reads and the permission each needs, and state the known limitation that grants made through a group are not seen.Proxy
fetch, which ignoresHTTPS_PROXY/HTTP_PROXYon its own, although the troubleshooting docs told users behind a proxy to setHTTPS_PROXY.proxyPolicy, which@azure/identityuses for its token requests) tofetchthrough anundiciglobal dispatcher, on every supported Node.js version: one proxy fromHTTPS_PROXY, thenALL_PROXY, thenHTTP_PROXY(upper case before lower case);NO_PROXYentries match an exact host or a leading-dot domain; proxy URL credentials are sent as BasicProxy-Authorization.undici^7.30.1 (Node.js 20.18.1 and later).User-visible changes
HttpError: HTTP <status>: [<code>] <message>: <target>: <detail>; …, with the details of the ARM error (nested ones included) in full, instead ofHttpError: HTTP <status>: <full body>. A body with no ARM error is cut to its first 500 characters, the lengthmain's API Center client used for operation failures. The full body stays onerror.body. For example, an invalid policy now readsHTTP 400: [ValidationError] One or more fields contain incorrect values: representation: The 'set-header' start tag on line 4 position 6 does not match the end tag of 'inbound'. Line 5, position 5.main's APIM name,HttpError, and is used for ARM calls and for the SAS links export actions return.ApicHttpErrorremains a deprecated alias, so imports andinstanceofchecks keep working, but API Center errors now print asHttpErrorinstead ofApicHttpError.fetch failed (getaddrinfo ENOTFOUND …)), wherever the CLI formats them witherrorMessage, not only for a network error after the last retry.HTTPS_PROXY(orALL_PROXY/HTTP_PROXY) set, ARM requests go through the proxy, chosen and bypassed as@azure/identitydoes.Timings
All retry and polling intervals and deadlines are the values
mainalready uses. The token refresh point is the SDK's (2 minutes before expiry, in the background) instead ofmain's 5 minutes, since token caching now usesgetBearerTokenProvider. Server-requested waits are read as the Azure SDK reads them and are not bounded, as in the SDK. Between polls, a positiveRetry-Aftersets the wait for every later poll until another replaces it, and a missing, unreadable,0or past value keeps the current wait. That is whatmain's APIM client and the SDK's LRO poller (@azure/core-lro) do;main's API Center client went back to 5 seconds when a poll had no header.Tests
tests/unit/lib/proxy.test.ts(includes afetchthrough a local proxy),tests/unit/clients/apic-client.test.ts(API Center had no client tests),arm-http-client.test.ts,preflight.test.ts,error-message.test.ts.Retry-After, polling) is tested once, inarm-http-client.test.ts, which takes overmain's APIM client tests of it; the client tests cover each client's own choices (URLs, statuses, intervals, deadlines, hosts, body release). The clients' pre-flight tests are consolidated intopreflight.test.ts, keeping three per client for its own wording: a possibly wrong resource group, the permission error, and the missing service.tests/unit/cli/index.test.tsruns the CLI to check the proxy is applied before a command runs.npm test: all pass except the 2 environment-relatedpackage-buildintegration tests that also fail onmain.npm run lintandnpm run buildare clean.Verified against Azure
Integration runs on 2026-11-04 and 2026-11-05 against a StandardV2 APIM source and target, an API Center service and a Key Vault in the public cloud, with
mainas the baseline:az login, and API Center extract and publish with a specification import: same output and target state asmain.Retry-Afterin seconds,retry-after-msbeforeRetry-After,Retry-Afteras a date (future and past), 429 without a header (1, 2, 4 seconds, then the status), 503 with and without a wait.--delete-unmatched, pagination over 120 tags, and anextLinkrewritten to another host (the list fails and no token goes there).HTTPS_PROXY, the right and a wrong password (fails at once with the 407 message, password never logged),NO_PROXYexact and domain matches, lower-case andALL_PROXY/HTTP_PROXYvariables, andNO_PROXY=*. The proxy being used,NO_PROXYexact matches and the wrong password ran on Node.js 22.0.0, 22.23.4 and 24.22.0; the other proxy cases on the default Node.js only.HTTP 403 from https://…blob.core.windows.net/api-export/<id>: <?xml…><Error><Code>AuthenticationFailed</Code>…, with no query or signature in the log.+in the signature, a JSON-escaped separator, asigvalue running only over the characters a signature can have, the JSON-escaped forms of the signature, and URLs percent-encoded twice or withsigfirst.Not verified
--client-id/--client-secret), and without Reader on the resource group (Support least-privilege RBAC for apiops publish — avoid requiring Resource Group Reader in preflight #303).Follow-ups (not in this PR)
Key Vault pre-flight permission checks — same behaviour as
maintoday:properties.keyVault.lastStatus.Related Issue(s)
Fixes #303