Skip to content

fix(vscode): Guard against bundle download corruption #9294

Open
lambrianmsft wants to merge 10 commits into
Azure:mainfrom
lambrianmsft:lambrianmsft/verify-dependency-downloads
Open

fix(vscode): Guard against bundle download corruption #9294
lambrianmsft wants to merge 10 commits into
Azure:mainfrom
lambrianmsft:lambrianmsft/verify-dependency-downloads

Conversation

@lambrianmsft

Copy link
Copy Markdown
Contributor

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Microsoft.Azure.Functions.ExtensionBundle.Workflows is downloaded from a CDN at extension activation. When the CDN occasionally returns a truncated zip, the extension cached the partial file and failed later in opaque ways. There was also no way to test against an unreleased bundle locally, no fallback when a private/experimental CDN was misconfigured, and no user-visible signal during activation downloads.

This PR adds end-to-end integrity verification (size + MD5), opt-in experimental bundle settings with a public-CDN safety net, and withProgress notifications so the user sees what is happening (and why a redownload is occurring).

Impact of Change

  • Users: Bundles are verified against Content-Length and Content-MD5 on download, and against an MD5 sidecar on every activation. Corrupt local copies are detected and replaced automatically. Users see a progress notification during downloads and a warning toast immediately before a corruption-triggered redownload, so silent activation hangs are gone. Three new VS Code settings (useExperimentalExtensionBundle, experimentalExtensionBundleSourceUri, experimentalExtensionBundleVersion) let users point at unreleased bundle builds without losing the public-CDN safety net.
  • Developers: New downloadFileWithVerification, verifyLocalBundleHash, fetchExpectedMd5, and isMissingPackageError helpers in integrity.ts. bundleFeed.ts gains a DownloadReason-driven withProgress wrapper used at every download call site, plus a private-to-public CDN fallback chain that only triggers on "package missing" errors (4xx / DNS), never on integrity failures. New E2E phase bundleintegrityonly exercised via run-e2e.js.
  • System: One extra HEAD request per activation to validate the local sidecar. No change to the public CDN contract. The MD5 sidecar lives next to the bundle so a manually-deleted bundle directory self-heals.

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in: VS Code extension host (Windows), public Azure CDN, simulated experimental source override. 66 bundleFeed unit tests + 1065 vscode-designer extension-unit tests passing. New bundleintegrityonly live E2E phase 4/4 passing.

Contributors

Screenshots/Videos

Copilot AI added 6 commits June 18, 2026 12:36
The extension bundle and other CDN-hosted dependencies were streamed to disk without verifying what arrived. Truncated or corrupted downloads silently extracted, mainly affecting Microsoft.Azure.Functions.ExtensionBundle.Workflows.

* Add downloadFileWithVerification: streams the file, counts bytes, computes MD5 inline, and asserts both against the response's Content-Length and Content-MD5 headers. Azure Blob Storage (which backs cdn.functions.azure.com) sets these on upload, so this catches CDN-edge corruption end-to-end with no publishing-pipeline changes.
* Wrap downloads in a retry loop (default 3 attempts, exponential backoff). Retries network errors, 5xx responses, and DownloadIntegrityError; surfaces 4xx as fatal.
* Fix latent bug in downloadAndExtractDependency: the axios.get(...).then(...) chain was never awaited, so callers' awaits resolved before the download started and write errors were not propagated. Now downloads complete before extraction, and errors bubble up to callers.
* Telemetry: <dep>DownloadAttempts, ExpectedSize, ActualSize, Md5Match (true/skipped), DownloadDurationMs per attempt.
* Treat missing Content-Length / Content-MD5 as advisory so non-Blob mirrors keep working.

Applies to all callers of downloadAndExtractDependency (extension bundle, Func Core Tools, Node.js, .NET install script).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rification, CDN E2E probe

Phase 2-4 of the bundle integrity work.

* Adds 3 azureLogicAppsStandard settings: useExperimentalExtensionBundle,
  experimentalExtensionBundleSourceUri, experimentalExtensionBundleVersion.
  When the toggle is on, downloadExtensionBundle never calls the public CDN
  for `latest'' so a local/preview bundle isn't silently overridden.
* Extracts a vscode-free integrity helper (app/utils/integrity.ts) so the
  same verifier is used by binaries.ts and the new E2E probe.
* On-disk hash verification: each download writes a sidecar
  .bundle-source-md5; on next startup we HEAD the public bundle zip and
  redownload if the sidecar is missing or stale.
* New E2E phase (bundleCdnHealth.test.ts, E2E_MODE=bundleintegrityonly):
  pure-Mocha probe that fails loudly if the CDN ever stops emitting
  Content-Length / Content-MD5. Wired into the independentonly shard.
* writeTestSettings() in run-e2e.js gains experimental-bundle options.
* SKILL.md and docs/ai-setup/packages/vs-code-designer.md updated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- bundleFeed: use membership test (localVersions.some(v === envVarVer))
  instead of equality with latestLocalBundleVersion so the env-var pin
  is honored when on disk alongside other versions.
- bundleFeed: add tryDownloadBundleAndWriteSidecar / tryFetchSourceIndex
  helpers that swallow 'package not present' errors (HTTP 4xx, network
  failures from isMissingPackageError) and surface them as fallback
  signals rather than fatal exceptions.
- bundleFeed: when toggle is on, the experimental sourceUri is now
  attempted first (with index probe to detect pinNotInIndex), then
  falls back to the public CDN for the same pin / latest. Both source
  and public failing throws so the dev sees the failure.
- bundleFeed: experimentalSourceFallback telemetry property records
  pinNotInIndex | zip404 | index404 | networkError reasons.
- bundleFeed: 'fall through to public' path now downloads from
  PUBLIC_BUNDLE_BASE_URL, not the broken experimental baseUrl.
- package.json: reword the three experimental settings to call out the
  master-toggle gating and the public-CDN safety net.
- integrity: export isMissingPackageError shared helper.
- bundleFeed.test: 8 new tests covering env-var membership + fallback
  matrix (zip404 / pinNotInIndex / index404 / networkError / both
  CDNs failing / toggle off ignores experimental settings).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…+ toasts

When the extension activates and decides to (re)download the Logic Apps
extension bundle, the user now sees a notification explaining what's
happening rather than waiting silently while activation hangs.

- bundleFeed: new downloadBundleWithProgress / tryDownloadBundleWithProgress
  helpers wrap every download call in vscode.window.withProgress with a
  context-specific title that names the version and the source CDN
  (public Azure CDN vs the configured private/experimental URL).
- bundleFeed: when verifyLocalBundleHash returns sidecarMissing /
  sidecarMismatch we now showWarningMessage *before* redownloading so
  the user understands why the bundle is being replaced even though
  they didn't change anything.
- bundleFeed: success toast (showInformationMessage) on completion.
- bundleFeed: all download call sites (env-var pin, experimental pin,
  experimental cold-start, public-CDN fallback, public newer-version,
  corrupt-local redownload) now go through the new helpers.
- bundleFeed.test: extend vscode mock with window.withProgress (calls
  task immediately), showWarningMessage, showInformationMessage, and
  ProgressLocation. Two new tests assert the corruption warning + the
  newer-version progress notification render with the expected copy.

64 → 66 bundleFeed tests, 1063 → 1065 extension-unit tests passing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Future sessions opening a PR must read .github/pull_request_template.md
(lowercase filename) before writing the body. Promote this from the
generic 'opening a PR body' row to its own top-of-table row, and add it
as Hard Rule #1 so it cannot be skipped.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…at applies

Add explicit rule to release-scribe pattern in review-patterns.md: when
authoring a PR body, copy the entire template structure (every section,
every checkbox, every HTML comment hint) and only flip applicable boxes
to [x]. Never delete unticked rows.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 18, 2026 22:57
@lambrianmsft lambrianmsft added the risk:medium Medium risk change with potential impact label Jun 18, 2026
@github-actions

github-actions Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(vscode): Guard against bundle download corruption
  • Issue: None. The title is specific and descriptive.
  • Recommendation: No change needed, though you may remove the trailing space.

Commit Type

  • Properly selected (feature) and only one checkbox is ticked.
  • Note: This is acceptable, though the change reads more like a fix than a feature based on the title/body.

Risk Level

  • The selected risk label/body match (risk:medium) and the label is present.
  • Assessment: Given the breadth of the change, dependency-download behavior, and fallback logic, I’d advise high risk rather than medium.

What & Why

  • Current: Clear and detailed.
  • Issue: None.
  • Recommendation: No change required.

Impact of Change

  • The section is well covered and gives a solid picture of user, developer, and system impact.
  • Recommendation:
    • Users: Good as written.
    • Developers: Good as written.
    • System: Good as written.

Test Plan

  • Passed. The diff includes both unit tests and E2E tests, which satisfies the template requirements.

⚠️ Contributors

  • The section is present but empty.
  • Recommendation: If others contributed ideas, reviews, or implementation, add them here with @mentions. If not, it can remain blank.

Screenshots/Videos

  • Not required for this PR type based on the diff.
  • Assessment: Blank is acceptable.

Summary Table

Section Status Recommendation
Title Remove trailing space if desired
Commit Type Consider whether fix better matches the change
Risk Level ⚠️ I advise risk:high instead of risk:medium
What & Why None
Impact of Change None
Test Plan None
Contributors ⚠️ Add contributors if applicable
Screenshots/Videos None

Overall, the PR passes review, but I recommend increasing the risk level label/body to high to better reflect the scope and behavioral impact of the change.


Last updated: Fri, 19 Jun 2026 01:27:06 GMT

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the VS Code designer extension’s activation-time download of Microsoft.Azure.Functions.ExtensionBundle.Workflows by adding integrity verification and improving the developer/testing story around experimental bundle sources. It also adds a new Mocha-only E2E phase to continuously validate the public CDN’s integrity headers and exercise the verifier logic without spinning up VS Code.

Changes:

  • Added CDN download integrity verification (size + MD5) plus a local MD5 sidecar to detect and self-heal corrupt cached bundles on subsequent activations.
  • Introduced opt-in experimental bundle settings with a “private source → public CDN” fallback that triggers only for missing-package style failures.
  • Added a Mocha-only E2E probe (bundleintegrityonly / Phase 4.11) and documented the new phase in the VS Code E2E skill docs.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
docs/ai-setup/packages/vs-code-designer.md Documents new E2E phase 4.11 and E2E_MODE=bundleintegrityonly.
apps/vs-code-designer/src/test/ui/SKILL.md Adds the new CDN integrity probe test to the VS Code E2E “skill” inventory and mode table.
apps/vs-code-designer/src/test/ui/run-e2e.js Adds a Mocha-only phase runner plus a fast-path mode for bundleintegrityonly, and wires the probe into independentonly.
apps/vs-code-designer/src/test/ui/bundleCdnHealth.test.ts New live CDN probe validating Content-Length/Content-MD5 on the bundle zip and a small verified download.
apps/vs-code-designer/src/package.json Adds three new VS Code settings for experimental bundle selection and source URI/version.
apps/vs-code-designer/src/constants.ts Adds setting keys and the MD5 sidecar filename constant.
apps/vs-code-designer/src/app/utils/integrity.ts New vscode-free download verifier + helper utilities (fetchExpectedMd5, retry logic, missing-package classification).
apps/vs-code-designer/src/app/utils/bundleFeed.ts Integrates verification, sidecar checks, experimental source resolution + fallback chain, and user-visible progress/toasts.
apps/vs-code-designer/src/app/utils/binaries.ts Routes all dependency downloads through the verifier and propagates integrity results/telemetry.
apps/vs-code-designer/src/app/utils/test/bundleFeed.test.ts Updates/adds unit tests for sidecar verification, progress/warning UX, and experimental fallback behavior.
apps/vs-code-designer/src/app/utils/test/binaries.test.ts Adds unit tests for downloadFileWithVerification behavior (MD5/size verification, retry rules, telemetry).
.squad/knowledge/review-patterns.md Tightens PR-body guidance: preserve template sections/checkboxes verbatim.
.squad/knowledge/INDEX.md Updates knowledge index triggers and hard rules around PR body editing.

Comment on lines +102 to +106
const headers = response.headers ?? {};
const contentLengthRaw = headers['content-length'] ?? headers['Content-Length'];
const expectedSize = contentLengthRaw === undefined ? undefined : Number.parseInt(String(contentLengthRaw), 10);
const expectedMd5Raw = headers['content-md5'] ?? headers['Content-MD5'];
const expectedMd5 = typeof expectedMd5Raw === 'string' && expectedMd5Raw.length > 0 ? expectedMd5Raw : undefined;
Comment on lines +169 to +172
export async function fetchExpectedMd5(url: string): Promise<string | undefined> {
const response = await axios.head(url);
const headers = response.headers ?? {};
const expectedMd5Raw = headers['content-md5'] ?? headers['Content-MD5'];
Comment on lines +671 to +675
if (hashCheck === 'sidecarMissing' || hashCheck === 'sidecarMismatch') {
// Surface a one-shot warning so the user understands *why* we're
// suddenly downloading on what looks like a steady-state activation.
notifyCorruptionDetected(latestLocalBundleVersion, describeSource(effectiveBaseUrl));
await downloadBundleWithProgress(context, effectiveBaseUrl, latestLocalBundleVersion, hashCheck);
Comment on lines +335 to +342
case 'sidecarMissing':
case 'sidecarMismatch':
return localize(
'bundleProgressRedownload',
'Re-downloading Logic Apps extension bundle {0} from {1} (local copy was incomplete)…',
version,
source
);
Comment on lines +1808 to +1814
if (e2eMode === 'bundleintegrityonly') {
// Pure Node Mocha — no ExTester, no VS Code session needed. Probes
// cdn.functions.azure.com to confirm the integrity headers we depend
// on are still emitted.
const exit = await runMochaPhase('Phase 4.11: bundleCdnHealth', phaseBundleIntegrityFiles);
process.exit(exit);
}
…esponses

Reported by user: every dotnet SDK install fails 3x with
  Download integrity check failed for https://dot.net/v1/dotnet-install.ps1:
  size mismatch (expected 24942, got 76680).

Root cause: dot.net/v1/dotnet-install.ps1 is served gzipped by the CDN.
axios (Node) sends Accept-Encoding: gzip, ... by default and auto-decompresses
the stream. Our Content-Length check then compared the gzipped header value
(24942) against the decoded bytes piped to disk (76680) and threw
DownloadIntegrityError on every attempt.

This is a real regression introduced by Phase 5: dotnet install + any text
dependency the CDN gzips at the edge has been broken since that ship. Bundle
zips (binary, not gzipped) were not affected, which is why E2E missed it.

Fix:
- integrity.ts: force Accept-Encoding: identity + decompress: false on the
  axios GET so Content-Length reflects the actual transferred bytes.
- integrity.ts: defensive — if the server returns Content-Encoding anyway,
  skip the size check (Content-Length is meaningless next to decoded bytes).
  MD5 is still verified when present.
- integrity.ts: same Accept-Encoding: identity on the fetchExpectedMd5 HEAD.

Tests:
- binaries.test.ts: 3 new cases — (a) gzip response with mismatched
  Content-Length succeeds, (b) un-encoded mismatched Content-Length still
  throws DownloadIntegrityError, (c) Accept-Encoding: identity header is
  sent on every request.
- bundleCdnHealth.test.ts: new live E2E case that round-trips
  dot.net/v1/dotnet-install.ps1 end-to-end via downloadFileWithVerification
  and sanity-checks the script body. Catches future CDN/axios drift on the
  actual URL we install from.

109 binaries+bundleFeed unit tests passing. 5/5 bundleintegrityonly live
E2E tests passing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI added 3 commits June 18, 2026 17:04
verifyLocalBundleHash only compared the stored sidecar string to the CDN's HEAD
Content-MD5, so any post-download mutation of the extracted bundle on disk (manual
edits, disk bit-rot, AV restore, partial overwrite, interrupted install) silently
passed verification. The user can end up running a corrupt 400 MB bundle indefinitely.

Upgrades the sidecar to JSON { version, sourceMd5, contentHash } and adds
computeBundleContentHash — a deterministic streaming sha256 over the extracted tree
(POSIX-normalized relative paths, sidecar self-excluded, <relPath>\\0<size>\\0<bytes>\\0
framing). verifyLocalBundle now recomputes that hash on every activation before it
falls through to the existing publisher-MD5 HEAD check.

New result 'contentMismatch' joins the existing Phase 6 corruption-notification path:
warning toast + progress notification + redownload. Legacy bare-MD5 sidecars (and JSON
sidecars missing contentHash) collapse into 'sidecarMissing' so the one-time migration
stays silent — those users haven't actually hit a corruption event.

Rejected alternatives (kept here for future readers):
- range-GET the zip central directory from the CDN: regresses offline activation
- keep source.zip on disk: ~400 MB per cached version, 1+ GB redundancy with 2-3 cached

Unit coverage: 7 new tests in bundleContentHash.test.ts (determinism, byte/add/remove
sensitivity, sidecar exclusion, path sensitivity) + 3 new full-pipeline cases in
bundleFeed.test.ts (legacy migration silent, JSON contentHash mismatch fires toast +
redownload, matching JSON sidecar stays 'passed'). 1078 unit tests pass, 5/5
bundleintegrityonly E2E pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…dle download

main.ts fires downloadExtensionBundle as a background task during activation so
the UI stays snappy. When the bundle is corrupt or missing, the re-extract can
take 10+ seconds — and if the user opens a workflow during that window,
startDesignTimeApi spawns func.exe against the half-extracted bundle folder.
On Windows that locks the folder, kills the design-time host, and leaves the
bundle in a half-extracted state until the next activation.

Observed in the wild:
  5:48:24 PM: Re-downloading Logic Apps extension bundle 1.165.52...
  5:48:28 PM: Starting Design Time Api...   ← user opened workflow
  5:48:29 PM: Stopping Design Time Api...   ← crashed
  Successfully downloaded ...               ← download finally finished

Fix: add a module-level in-flight tracker in bundleFeed.ts. downloadExtensionBundle
dedupes concurrent callers and exposes:
  - waitForExtensionBundleReady(): blocks on the in-flight work (noop otherwise)
  - isExtensionBundleDownloadInFlight(): sync probe for log-gating

startDesignTimeApi awaits waitForExtensionBundleReady() immediately before spawning
func.exe, with a one-line output-channel message so the user sees what's happening.
main.ts also gets a .catch() so background download failures surface in the log
instead of becoming unhandled rejections.

New unit test: concurrent downloadExtensionBundle calls dedupe (only one
downloadAndExtractDependency call), waitForExtensionBundleReady blocks until that
call completes, and the in-flight flag clears afterward. 1079/1079 unit tests pass.
5/5 bundleintegrityonly E2E pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
After downloadFileWithVerification confirms the zip itself, AdmZip.extractAllTo
could leave a partial tree on disk if a file lock, antivirus quarantine, or disk
pressure interrupted extraction mid-stream. We then wrote a content-hash sidecar
over the partial tree, blessing the broken state as 'good'. Subsequent
activations passed the sidecar check while func.exe failed to load missing
assemblies.

Phase 11 closes the gap:

- New verifyExtractedZip walks the zip's central directory after extraction and
  asserts every non-directory entry exists on disk at the expected size. Throws
  BundleExtractionError with a discriminator (missing | sizeMismatch | empty).
- extractDependency wraps cleanDirectory + extractAllTo + verifyExtractedZip in
  a 2-attempt retry loop that absorbs transient EBUSY/EPERM/EACCES locks and
  verification failures with a 750ms backoff.
- downloadAndExtractDependency now tears down the partial targetFolder if
  extraction throws, so the next activation re-downloads from scratch instead
  of inheriting a corrupt tree.
- downloadBundleAndWriteSidecar refuses to write a sidecar when the content
  hash is undefined (bundleDir vanished post-extract).

Tests: 5 new unit cases in binaries.test.ts covering happy path, missing file,
truncated file, empty extract dir, and empty zip. 1084/1084 vscode-designer
unit tests pass; bundleintegrityonly E2E (5/5) green.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:medium Medium risk change with potential impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants