Authenticate GitHub marketplace API requests via RFC 8693 token exchange - #325815
Draft
Michael Cummings (MSFT) (mcumming) wants to merge 32 commits into
Draft
Authenticate GitHub marketplace API requests via RFC 8693 token exchange#325815Michael Cummings (MSFT) (mcumming) wants to merge 32 commits into
Michael Cummings (MSFT) (mcumming) wants to merge 32 commits into
Conversation
…ntext key Introduce the `extensions.gallery.authProvider` policy that selects which identity provider (github or microsoft) gates Private Marketplace access, and register it in the exported policy data. Add the marketplace auth-provider context key and the Entra ID resource scope constant used to acquire a Private Marketplace-audienced token. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Resolve the marketplace access strategy in the workbench gallery manifest service: cache-first startup, provider-routed access handling, Microsoft eligibility probing against the eligibility resource from the gallery manifest, the GitHub DefaultAccount path, the marketplace auth-provider context key, and access telemetry. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Surface a provider-aware sign-in prompt and access-denied state in the extensions viewlet, driven by the marketplace auth-provider context key so the correct identity provider is presented to the user. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Allow the built-in extensions gallery to silently use Microsoft (Entra ID) authentication sessions for Private Marketplace access. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Cover provider selection, cache-first startup, Microsoft eligibility handling, and the GitHub access path in the gallery manifest service. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ndling Address rubber-duck review findings on the Entra ID marketplace path: - Scope the cached access verdict to the marketplace it was computed against (authProvider + accountId + serviceUrl), rejecting stale caches on any mismatch. - Guard cache application and background validation with a monotonic epoch so a session/account/config change mid-validation supersedes an in-flight result. - Register session/account listeners before applying the cache, and the config listener before initial validation, closing startup TOCTOU windows. - Route transient auth-service and marketplace-fetch failures to Unreachable instead of leaving a configured marketplace on a blank Unavailable view. - Split 401 (missing/expired token -> RequiresSignIn, not cached) from 403 (durable denial -> AccessDenied, cached ineligible). - Never follow redirects on token-bearing requests; only send the Entra token to an HTTPS same-origin target; reject non-2xx and non-manifest 200 responses before parsing. - Restore the galleryservice:custom:marketplace telemetry on the GitHub path and drop the unused server-provided eligibility reason from persisted cache. Expand unit coverage to 45 tests across provider routing, eligibility, caching, error classification, and the epoch race paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…g, resource validation, UX copy - Policy: make the `extensions.gallery.authProvider` schema enum and enumDescriptions unconditional (`github`, `microsoft`). Gating the enum on the Entra product flag left the policy metadata exporting two enum descriptions against a single-value enum, which fails the policy-artifact generator's equal-length requirement on a clean export. The Entra gate is already enforced at runtime in getEffectiveAuthProvider(), and the setting is hidden (included: false), so this advertises nothing new in the UI. - Cross-account authorization leak: on Microsoft session change and GitHub default-account change, revoke the active manifest (drop `Available`) before revalidating. Previously the active status stayed `Available`, so a transient index/eligibility failure on the new account preserved the prior account's access. - Layering: move CONTEXT_MARKETPLACE_AUTH_PROVIDER down to the platform extensionGalleryManifest module so the workbench service no longer imports from a workbench/contrib module. The Extensions contribution re-exports it for existing consumers. - Resource validation: reject a 200 service index whose `resources` entries are malformed (missing string `id`/`type`), not just a non-array `resources`. Endpoint discovery calls `resource.type.split()` outside the fetch try/catch, so an undefined `type` would crash initialization instead of surfacing `Unreachable`. - UX: make the Microsoft AccessDenied welcome message generic. A bare 403 gives no typed reason, so asserting that an Entra ID account or Visual Studio Subscription is required could tell an already-signed-in user to obtain access they already have. Adds a unit test covering the malformed-resources -> Unreachable path. All 47 gallery tests pass; typecheck-client and valid-layers-check are clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Introduce `discoverMarketplaceProtectedResource` and supporting types/URIs that read a marketplace's well-known `/.well-known/oauth-protected-resource` document (RFC 9728): the advertised `resource`, `authorization_servers` and `scopes_supported`. Discovery is driven by the metadata body (CORS-readable) rather than the `WWW-Authenticate` challenge header, which the renderer's cross-origin index fetch usually cannot read. This is the reusable primitive the window process uses to negotiate a resource-scoped token for a gated index. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
When the marketplace service index is `[Authorize]`-gated it answers the initial (plain sign-in token) read with a 401. `fetchServiceIndexNegotiated` now discovers the marketplace's Protected Resource Metadata (RFC 9728), silently acquires a token bound to the advertised authorization server and resource scopes (RFC 8707) and retries the index once with that token. The negotiated token is captured in `negotiatedAccessToken` (set only on the eligible -> Available transition, cleared on every non-Available transition via `update(null, ...)`) and exposed through a new `getAccessToken()` so protected marketplace requests can authenticate. The `WWW-Authenticate` challenge is captured as a best-effort hint but discovery does not depend on it. Includes tests for gated-index negotiation, a CORS-stripped 401 (no challenge header), the open-index no-token case, and a negotiated-but-forbidden retry (403 -> AccessDenied). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Thread the resource-scoped token (from `IExtensionGalleryManifestService.getAccessToken`) onto the marketplace requests the gallery service initiates - extensionquery, asset and VSIX downloads - via `getMarketplaceAuthorizationHeader`. The token is only attached to same-secure-origin targets (`isSameSecureOrigin` guard) so it is never leaked cross-origin or over cleartext. For an open marketplace `getAccessToken` returns undefined and requests stay anonymous. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The shared process and remote server never negotiate the resource-scoped token themselves, so protected requests they initiate - extension `getManifest` and VSIX download - would be anonymous and rejected with 401. Push the token over the manifest channel alongside the manifest (`setExtensionGalleryManifest(manifest, accessToken)`) and expose it via the IPC service's `getAccessToken`. The token is coherent with the manifest: a null manifest always arrives with an undefined token, so a stale token can never outlive its access. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The extension resource loader fetches marketplace-hosted assets (README images, web README/CHANGELOG). `getExtensionGalleryRequestHeaders` now merges the negotiated token onto those requests behind the same same-origin guard, so resources on a gated marketplace load instead of 401ing. Anonymous for an open marketplace. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
When the user signs in from the marketplace-access welcome view, request the resource-scoped scopes so the first post-sign-in validation already holds a token the gated index accepts, avoiding a second silent negotiation round-trip. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
`onDidChangeSessions('microsoft')` also fires on routine token refreshes for the
same account; previously each one cleared the cache and forced a redundant
eligibility POST (and a manifest flash). Now the handler just re-validates, and
`handleMicrosoftAccess` resolves the current account and skips the eligibility POST
when a durable verdict for that account+marketplace is already cached - it still
(re)negotiates the service-index token for the session. A verdict for a different
account (account switch) revokes the prior authorization before the async round-trip
so a transient failure cannot preserve it.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A gated service index answers the initial read with a 401 + `WWW-Authenticate` as the first step of RFC 9728 negotiation - an expected handshake, not a failure. Logging it at `error` surfaced a spurious "Error retrieving extension gallery manifest". Log `MarketplaceAuthRequiredError` outcomes at `trace` and reserve `error` for genuinely unexpected failures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
When the marketplace index is `[Authorize]`-gated (RFC 9728 401), the GitHub auth
scheme now negotiates a resource-bound bearer token by exchanging the user's existing
GitHub session token at the marketplace's advertised authorization server (RFC 8693),
then retries the index and every protected API request presenting that token.
- Add `exchangeMarketplaceResourceToken` (extensionGalleryManifest.ts): discovers the
AS metadata (RFC 8414), guards the token endpoint with a same-origin HTTPS check, and
POSTs the token-exchange grant (subject_token = GitHub session token, resource =
marketplace, scope = access_as_user). The raw GitHub token is only ever sent to the AS
token endpoint, never to the resource server; `followRedirects: 0` prevents leaking it.
- Generalize `fetchServiceIndexNegotiated` to take an `acquireToken` callback so the
Microsoft (MSAL resource token) and GitHub (token exchange) paths share the 401 ->
discover PRM -> acquire token -> retry negotiation.
- Rework the GitHub eligible branch to negotiate: open index -> Available (no bearer);
gated index -> exchange -> Available with the negotiated token exposed via getAccessToken;
403 -> AccessDenied (cached ineligible); 401 -> RequiresSignIn (not cached). No
server-side eligibility POST is made on the GitHub scheme.
- Add `resolveGitHubSubjectToken`/`acquireGitHubResourceToken` helpers and re-validate on
`onDidChangeSessions('github')` so a session appearing recovers from RequiresSignIn.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Cover the GitHub auth-enabled scheme end to end against the mocked request/auth stack:
- auth-disabled (open index) -> Available, no bearer sent, no token exchange
- auth-gated index -> PRM discovery + token exchange -> Available, negotiated token
exposed; asserts the RFC 8693 grant body (grant_type, subject_token, resource, scope)
and that no /eligibility POST is made on the GitHub scheme
- gated index, no GitHub session (no subject token) -> RequiresSignIn (not cached)
- gated index, negotiated token still forbidden (403) -> AccessDenied (cached ineligible)
- token exchange rejected by the AS (400) -> RequiresSignIn (not cached)
- onDidChangeSessions('github'): a session appearing recovers from RequiresSignIn
Adds GitHub session/PRM/AS-metadata/token-endpoint test fixtures and wires a
`githubSessions` stub into the authentication mock.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Michael Cummings (MSFT) (mcumming)
July 14, 2026 15:35
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Adds RFC 8693 GitHub token exchange for authenticated Private Marketplace deployments, alongside stacked Entra authentication groundwork.
Changes:
- Negotiates and distributes resource-scoped marketplace tokens.
- Attaches tokens to protected gallery and resource requests.
- Adds provider-aware UI, policy configuration, caching, and tests.
Show a summary per file
| File | Description |
|---|---|
build/lib/policies/policyData.jsonc |
Exports the auth-provider policy. |
product.json |
Registers Microsoft trusted auth access. |
src/vs/base/common/product.ts |
Adds the Entra product gate. |
src/vs/platform/extensionManagement/common/extensionGalleryManifest.ts |
Defines auth metadata, token exchange, states, and APIs. |
src/vs/platform/extensionManagement/common/extensionGalleryManifestService.ts |
Provides the default token implementation. |
src/vs/platform/extensionManagement/common/extensionGalleryManifestServiceIpc.ts |
Transfers negotiated tokens over IPC. |
src/vs/platform/extensionManagement/common/extensionGalleryService.ts |
Authenticates gallery API and asset requests. |
src/vs/platform/extensionResourceLoader/common/extensionResourceLoader.ts |
Adds guarded resource authorization headers. |
src/vs/platform/extensionResourceLoader/common/extensionResourceLoaderService.ts |
Authenticates native resource requests. |
src/vs/platform/extensionResourceLoader/browser/extensionResourceLoaderService.ts |
Authenticates browser resource requests. |
src/vs/workbench/contrib/extensions/common/extensions.ts |
Re-exports the provider context key. |
src/vs/workbench/contrib/extensions/browser/extensionsViewlet.ts |
Adds provider-aware marketplace status UI. |
src/vs/workbench/contrib/extensions/browser/extensions.contribution.ts |
Registers policy and provider-routed sign-in. |
src/vs/workbench/services/extensionManagement/electron-browser/extensionGalleryManifestService.ts |
Implements negotiation, eligibility, caching, and invalidation. |
src/vs/workbench/services/extensionManagement/test/electron-browser/extensionGalleryManifestService.test.ts |
Tests provider and negotiation scenarios. |
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 5
- Review effort level: Medium
| if (!marketplaceApi || !AbstractExtensionGalleryService.isSameSecureOrigin(targetUrl, marketplaceApi)) { | ||
| return {}; | ||
| } | ||
| return { Authorization: `Bearer ${token}` }; |
| if (await this.isExtensionGalleryResource(uri)) { | ||
| const headers = await this.getExtensionGalleryRequestHeaders(); | ||
| const headers = await this.getExtensionGalleryRequestHeaders(uri); | ||
| const requestContext = await this._requestService.request({ url: uri.toString(), headers, callSite: 'extensionResourceLoader.readExtensionResource' }, CancellationToken.None); |
| // Auth service responded: account exists but ineligible → cache the result | ||
| this.cacheAccess({ authProvider: 'github', accountId: account.accountName, eligible: false, serviceUrl: configuredServiceUrl }); | ||
| this.update(null, ExtensionGalleryManifestStatus.AccessDenied); | ||
| } else if (this.currentStatus !== ExtensionGalleryManifestStatus.Available) { |
Comment on lines
+225
to
+227
| private get authenticationService(): IAuthenticationService { | ||
| return this._authenticationService ??= this.instantiationService.invokeFunction(accessor => accessor.get(IAuthenticationService)); | ||
| } |
| policy: { | ||
| name: 'ExtensionGalleryAuthProvider', | ||
| category: PolicyCategory.Extensions, | ||
| minimumVersion: '1.121', |
requestService followed 3xx redirects while re-sending all original headers, so an Authorization bearer/basic secret bound to the origin could be forwarded to a different host the redirect points at (e.g. a marketplace API redirecting an asset download to a third-party CDN). Cross-origin redirects now drop origin credential headers (case-insensitive Authorization) plus user/password, matching WHATWG fetch and curl; same-origin redirects keep them so legitimate intra-origin auth flows still work. Proxy-Authorization is preserved (it authenticates to the forward proxy, not the origin). Unparseable targets fail safe as cross-origin. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
fetchAuthorizationServerMetadata trusted the token and authorization endpoints from any well-known response without checking that the returned issuer matched the authorization server the discovery URL was built for. A compromised or misconfigured well-known endpoint could thus return metadata bound to a different issuer. Per RFC 8414 3 the issuer MUST be identical to the requested authorization server; mismatches are now rejected (failing closed so remaining discovery URLs are tried and, if none match, the caller sees an error). A single trailing slash is tolerated for deployments that normalize it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
Three fixes to the GitHub-authenticated marketplace path: - Refresh in place while Available: a GitHub session change (token rotation) that fires while the marketplace is already live now re-mints the RFC 8693 resource token via a new refreshNegotiatedGitHubToken helper instead of skipping (handleGitHubAccess is gated on currentStatus !== Available). It never re-publishes the manifest (no view flash) and never tears down access on a failed refresh, so a rotated subject token can't strand the session on a stale token. - Cancel the token exchange when superseded: fetchServiceIndexNegotiated now takes an isCurrent() guard and throws CancellationError after PRM discovery and after token acquisition if a sign-out/account-switch bumped the validation epoch, so a just-revoked GitHub subject token is never POSTed to the marketplace authorization server. Callers' epoch guards discard the cancellation. - Tri-state entitlement eligibility: checkAccess returns eligible | ineligible | unknown. entitlementsData undefined (entitlements endpoint unreachable/indeterminate) is unknown and surfaces a retryable Unreachable instead of a cached AccessDenied; only a resolved non-marketplace SKU (or null) is a durable ineligible. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
…origin redirects
The redirect handler validated the origin of the resolved URL but then re-requested the raw 'location' header, so a relative ('/path') or protocol-relative ('//host') redirect could send Authorization to an origin the cross-origin check never saw. Resolve the location once against the current URL, reject non-HTTP(S) targets, and use that resolved absolute URL for the follow-up request. Also strip Cookie and Cookie2 (not just Authorization) on cross-origin hops.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
…llation before token exchange The issuer check in fetchAuthorizationServerMetadata applied to every consumer, which breaks multi-tenant providers (e.g. Entra '/common', whose metadata returns a per-tenant issuer). Make it opt-in via 'validateIssuer' with an exact match (no trailing-slash normalization) and enable it only from exchangeMarketplaceResourceToken. Also recheck token.isCancellationRequested after the AS-metadata GET so a sign-out mid-discovery does not POST a now-revoked subject token. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
… and cover GitHub Enterprise Background token refreshes (GitHub session rotation, matched-verdict Microsoft re-validation) set negotiatedAccessToken in place but only pushed it over the IPC channel via updateChannels(manifest), which the Available->Available path skips — so the shared process kept authenticating with a token that would eventually expire (401 on getManifest/VSIX download). Add a setAccessToken channel command and an updateNegotiatedAccessToken helper that pushes the fresh token without republishing the manifest, and call it at both in-place refresh sites. Also match the 'github-enterprise' provider (not just 'github') in the session-change listener so GHE-backed default accounts get the same in-place refresh; the subject-token resolution was already provider-agnostic. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
…ketplace denial
checkAccess mapped entitlementsData null to ineligible, which the GitHub path caches as a durable AccessDenied. But defaultAccount.ts sets null only on a 401 (token expired/revoked) or 404 (the account lacks the scope to query entitlements) — both recoverable/indeterminate, not a definitive 'not entitled' verdict. Treat null like undefined ('unknown'), so it surfaces a retryable Unreachable message and is never cached as a denial that locks the user out until the cache is cleared. 'unknown' still never grants access, so this cannot over-authorize.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
The onDidChangeDefaultAccount listener cleared the cached verdict and revoked the manifest (a brief Unavailable flash) on every event. Because the default account also fires this event on routine same-account data refreshes (rotated session token, re-fetched entitlements), a signed-in eligible user saw the marketplace flash and a redundant revalidation whenever their account data refreshed. Track the resolved account identity (provider:session:accountName, excluding volatile token/entitlement data) in handleGitHubAccess and only pre-clear the cache and revoke the manifest when that identity actually changes or disappears (a real account switch or sign-out). Same-identity refreshes now revalidate in place with no flash and no cache drop. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
… expires
The negotiated GitHub resource token (RFC 8693 exchange) is short-lived, but
nothing tracked its lifetime: it was only ever refreshed reactively on a GitHub
session change. If the token expired while the session stayed put, the
marketplace kept sending an expired bearer and wedged until a window reload.
Carry the token's advertised lifetime through the negotiation and schedule a
proactive re-mint before it expires:
- exchangeMarketplaceResourceToken now returns { accessToken, expiresInSeconds }
(from the token response's expires_in) instead of a bare string, and the
window service threads expiresInSeconds through fetchServiceIndexNegotiated
and acquireGitHubResourceToken.
- On every successful gated-index negotiation (initial and background refresh),
arm a single timer to re-mint at GITHUB_TOKEN_REFRESH_FRACTION (0.75) of the
advertised lifetime, clamped to [1m, 6h]. When the server omits expires_in we
fall back to a conservative default lifetime rather than letting the token
expire silently.
- A failed proactive re-mint no longer gives up: it re-arms on a capped
exponential backoff (30s -> doubling -> 5m) so the token self-heals once
connectivity/identity is restored, with no window reload. Re-arming is guarded
on the validation epoch, an Available status, and a still-set negotiated token
so a concurrent sign-out/account switch wins.
- The Microsoft path returns { token } with no lifetime: MSAL owns refresh of
its own sessions via getSessions/onDidChangeSessions, so no scheduling is
needed there.
- update(null) is the single teardown choke point: it clears the timer and
resets the backoff alongside the negotiated token, so the schedule never
outlives the access that produced it.
The timer fires with a freshly bumped epoch (matching the reactive
session-change refresh) and is wrapped in a protected scheduleGitHubTokenRefresh
Timeout seam so tests drive the schedule deterministically.
This addresses the proactive-refresh (#4a) and self-healing (#4b) rubber-duck
findings. A fully reactive 401 re-negotiation (a shared-process -> window
reverse IPC channel that re-mints on demand when an API call 401s) is
intentionally deferred: proactive re-mint plus capped-backoff retry keeps the
bearer fresh and recovers a wedged marketplace without the added reverse-channel
complexity, and the existing session-change path still covers identity churn.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
The desktop caller of exchangeMarketplaceResourceToken passes CancellationToken.None, so the pre-POST token.isCancellationRequested guard could never fire in production: a sign-out or account switch during the authorization-server metadata GET still POSTed the (now stale) GitHub subject token to the token endpoint. Add an optional isCurrent() predicate to exchangeMarketplaceResourceToken that is checked alongside cancellation just before the POST. Thread it through acquireGitHubResourceToken and both negotiation call sites, which pass () => this.validationEpoch === epoch so a superseded validation aborts before the subject token leaves the client. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
The desktop window registers NativeRequestService, which routes through base/parts/request/common/requestImpl.ts. That request() built its RequestInit without ever setting `redirect`, so fetch defaulted to redirect:'follow' and the followRedirects:0 hardening (already applied to the node request service) was silently ignored in the window. A 307/308 from the marketplace token endpoint would then replay the subject-token POST body to the redirect target (form-urlencoded bodies are CORS "simple requests", so the body is sent even if the response is opaque). Map followRedirects === 0 to fetch's redirect:'manual'. An opaque redirect surfaces as status 0, which the token exchange (statusCode !== 200) already treats as a failure — fail-closed. Add real-transport tests over a live server: one documents that a 307 is followed by default (body replayed), the other asserts followRedirects:0 does not reach the target. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
The proactive re-mint of the negotiated GitHub marketplace token used a single one-shot timer. On several non-fatal paths the fired timer was silently dropped instead of being re-armed, so the resource token could expire and wedge the marketplace until a window reload: - A transient identity blip (`getDefaultAccount()` momentarily returns no account, or `checkAccess` -> 'unknown' entitlements) took the compound early-return and never rescheduled. - A failed re-mint only re-armed inside the catch; the same guard is now a shared helper. Restructure `refreshNegotiatedGitHubToken`: - Split the compound early-return so a superseded epoch still drops the schedule to the new owner, but a soft blip re-arms on the capped backoff via the new `rearmGitHubTokenRefreshAfterFailureIfLive` helper (epoch unchanged + still Available + token present). - When the index stops negotiating a token (admin reopened a gated index), clear the stale resource token via `updateNegotiatedAccessToken(undefined)` so the removal propagates to the shared process, and stop the schedule. - The catch re-arms through the same helper. Test: a transient blip on a live, token-backed marketplace re-arms the proactive timer (pendingRefreshes -> [30_000]) instead of dropping the fired one-shot (which would leave [1_350_000]). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
When `extensions.gallery.serviceUrl` or `extensions.gallery.authProvider` changes, the service bumps the validation epoch, clears the persisted cache, and prompts for a restart. That prompt is dismissable, so if the user declines, the PREVIOUS config's live manifest, negotiated resource token, and proactive-refresh timer kept running against the marketplace the admin just abandoned — the old gallery stayed Available and its token kept being re-minted until the window was eventually reloaded. Route the config-change handler through `update(null)` after clearing the cache. That is the single teardown choke point: it drops the negotiated token, cancels the proactive-refresh timer, resets the backoff, publishes a null manifest, and moves the status to Unavailable. The marketplace now goes Unavailable pending the (still-prompted) restart, which re-runs validation for the new config. Test: a GitHub auth-gated index negotiates a token (Available, timer armed); firing a config change drives status -> Unavailable, clears the negotiated token, and cancels the proactive refresh timer. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
The proactive re-mint delay for the negotiated GitHub resource token had two defects in `scheduleGitHubTokenRefresh`: 1. The minimum-delay floor was a coarse 60s. A genuinely short advertised lifetime (e.g. `expires_in: 30`) computed 75% = 22.5s but was clamped UP to 60s — firing the re-mint ~30s AFTER the token had already expired, so the marketplace would briefly present a dead bearer. 2. A present-but-invalid `expires_in` (0, negative, or non-finite) was conflated with an ABSENT one via `expires_in && expires_in > 0`, so an already-expired advertisement fell back to the conservative default hour and scheduled the re-mint ~45 minutes out. Extract a pure `computeRefreshDelay(expiresInSeconds)`: - omitted (`undefined`) -> conservative default lifetime, refresh at 75%; - finite positive -> refresh at 75% of it (short lifetimes refresh SOON, never clamped past expiry); - present but non-positive / non-finite -> refresh on the small hot-loop floor, not a default hour. Lower `GITHUB_TOKEN_MIN_REFRESH_MS` from 60s to a 5s hot-loop floor (it only guards against a near-zero lifetime busy-spinning the exchange, it is not a typical interval). `expires_in: 1800 -> 1_350_000` is unchanged. Tests: `expires_in: 30 -> 22_500` (before expiry, not the old 60s floor); `expires_in: 0 -> 5_000` (immediate, not a default-hour schedule). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
discoverMarketplaceProtectedResource read the resource_metadata URL out of the server-controlled WWW-Authenticate challenge and passed it straight to fetchResourceMetadata, so a compromised or malicious marketplace index could steer Protected Resource Metadata discovery at an attacker-controlled origin (SSRF GET) or a cleartext endpoint. Honor the challenge hint only when it is same-origin HTTPS with the configured service index; otherwise drop it and fall back to well-known discovery, which derives the PRM URL from the trusted service index origin. Fail closed on any parse error. Add a module-private isSameOriginHttpsUrl helper (platform/common, uses only the global URL) rather than threading the electron-browser isSafeTokenTarget guard across the layer boundary, since the browser-layer caller has no access to it. Add common tests asserting a cross-origin and a cleartext resource_metadata hint are never fetched (discovery falls back to same-origin well-known) and a same-origin HTTPS hint is honored. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eecf2b5c-ea9d-4c63-a708-45e5fb98e2a7
joshspicer
reviewed
Jul 27, 2026
Comment on lines
+176
to
+180
| // IPC channels to the shared process and (optionally) the remote server. The manifest and the | ||
| // negotiated resource token are pushed over these so those processes — which never negotiate a | ||
| // token themselves — can authenticate the protected marketplace requests they initiate | ||
| // (extension getManifest, VSIX download). | ||
| private readonly channels: IChannel[] = []; |
Member
There was a problem hiding this comment.
This sticks out to me a bit as unorthodox (or at least, I am unfamiliar with this pattern)
Member
|
TylerLeonhardt some oauth changes here, if interested in reviewing. |
Member
Karthik Nadig (karthiknadig)
requested a review
from Sandeep Somavarapu (sandy081)
August 5, 2026 15:57
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #325804 (which is itself stacked on #325331). Because those are cross-fork PRs whose head branches live only in
mcumming/vscode, GitHub won't let this PR base on the #325804 branch, so it bases onmainand the commit range below currently includes #325331's and #325804's commits. It will reduce to just the two GitHub-auth commits once those merge.Completes the GitHub auth-enabled portion of #325412 (the Entra path is covered by #325804).
Summary
#325804 made the marketplace's API requests authenticate for the Entra/Microsoft provider by negotiating a resource-scoped bearer token via RFC 9728 Protected Resource Metadata (PRM) discovery. This PR does the same for the GitHub provider.
The difference is token acquisition. VS Code's Microsoft/MSAL provider mints a resource-scoped token directly (
getSessions(..., { authorizationServer }), RFC 8707). VS Code's GitHub provider has no resource-token support, so when the marketplace index is[Authorize]-gated (RFC 9728401) the client exchanges the user's existing GitHub session token at the marketplace's advertised Authorization Server using the RFC 8693 token-exchange grant. The AS returns a first-partyat+jwtbound to the marketplace resource (aud = resource), which is then presented on the index retry and every protected API request.The raw GitHub token is only ever sent to the AS token endpoint, never to the resource server; both the AS and its discovered token endpoint are validated with a same-origin-HTTPS guard (fail-closed) before the token is sent, and
followRedirects: 0prevents a redirect from forwarding it elsewhere. Unlike the Entra scheme there is no server-side eligibility verdict on the GitHub path — access is decided entirely by the existing client-sidecheckAccess()gate; there is no/eligibilityPOST.Commits (the GitHub-auth work; #325331's and #325804's commits also appear in the range until they merge)
exchangeMarketplaceResourceToken(AS-metadata discovery + guarded token-exchange POST), generalizesfetchServiceIndexNegotiatedto a sharedacquireTokencallback, reworks the GitHub eligible branch to negotiate (open index → Available no-bearer; gated → exchange → Available with the token exposed viagetAccessToken();403→ AccessDenied cached;401→ RequiresSignIn not cached), and re-validates ononDidChangeSessions('github')./eligibilityPOST is made), no-session, negotiated-but-forbidden403, exchange rejected by the AS, and session-appears recovery from RequiresSignIn.How it maps to #325412
checkAccess()retained, no/eligibilityPOST.onDidChangeSessions('github')invalidation for the auth-enabled path.401/ no session / session +checkAccess, plus exchange failure and 403).Testing
extensionGalleryManifestService.test.ts— the six GitHub scenarios above (55 gallery tests passing total).typecheck-client✅ andvalid-layers-check✅.Open questions for review
client_idon the exchange. The exchange currently sends noclient_id. The server contract under-specifies the client side of the token endpoint, and OpenIddict token endpoints often require aclient_id. This needs validation against a live GitHub-auth-enabled deployment; if required, a product-config override for the exchangeclient_idwould be a small follow-up. This is the main reason the PR is opened as a draft.at+jwthas its own independent lifetime, so this is safe.Notes
getAccessToken()returnsundefined, and all requests stay anonymous — no behavior change.