Repository navigation
fix: harden SQL Server 2025 installer downloads - #296
cheenamalhotra wants to merge 1 commit into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation is focused and well tested, with only a non-blocking import-order issue.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Hardens SQL Server 2025 media downloads and documents/tests the supported workflow.
Changes:
- Isolates SSEI downloads, quotes paths, and validates installer output.
- Adds SQL Server 2025 documentation and CI coverage.
- Expands tests for aliases, caching, paths, and failures.
| File | Description |
|---|---|
src/utils.ts |
Hardens SSEI media handling. |
test/utils.ts |
Tests download behavior and errors. |
test/install.ts |
Tests cached SSEI media. |
action.yml |
Documents supported versions. |
README.md |
Adds SQL Server 2025 guidance. |
.github/workflows/release.yml |
Adds SQL 2025 integration coverage. |
.github/copilot-instructions.md |
Documents isolated media handling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
dhensby
left a comment
dhensby
left a comment
There was a problem hiding this comment.
Thanks @cheenamalhotra, this looks good.
Downloading the SSEI media into its own temp directory and requiring exactly one installer is sensible hardening. CI also confirms the bootstrapper accepts the quoted /MediaPath on a real runner: the windows-2025/sql-2025 job ran with /MediaPath="D:\a\_temp\sqlserver-media-3KLYYW" and got a single SQLServer2025-STDDEV-x64-ENU.exe. Adding sql-2025 to the matrix also restores our existing convention of testing sql-latest alongside each explicit version, which got missed when 2025 support was added.
I've left a couple of small docs nits inline. There are also two suggestions for bringing the rest of the code in line with what you've done here: quoting the other install paths, and isolating the box installer's extraction in the same way. Either can be done here or in a follow-up, whichever you prefer.
Heads-up: this conflicts with #295 (README, the node:fs/promises import in src/utils.ts, the module mocks in test/utils.ts, and lib/). Whichever PR lands second will need a rebase and a fresh npm run build.
| The action downloads the installation media using Microsoft's SSEI bootstrapper | ||
| and caches the extracted installer for subsequent runs. |
There was a problem hiding this comment.
nit: GitHub-hosted runners start every job on a fresh VM, so the cached installer is only reused when the runner persists (e.g. self-hosted runners). Maybe soften this to say it attempts to cache?
| The action downloads the installation media using Microsoft's SSEI bootstrapper | |
| and caches the extracted installer for subsequent runs. | |
| The action downloads the installation media using Microsoft's SSEI bootstrapper | |
| and attempts to cache the extracted installer for subsequent runs. |
| **Installer abstraction:** `src/installers/` contains a base `Installer` class and `MsiInstaller` subclass used by the native client and ODBC installations. SQL Server itself uses direct exe/box download logic or the SSEI bootstrapper (for 2025+) in `src/utils.ts`. | ||
|
|
||
| **Version registry:** `src/versions.ts` defines a `Map<string, VersionConfig>` with download URLs (exe/box or SSEI), optional update URLs, and OS compatibility constraints for each supported SQL Server version (2008–2025). SQL Server 2025+ uses the SSEI bootstrapper model (`sseiUrl`) instead of direct exe/box downloads. | ||
| **Version registry:** `src/versions.ts` defines a `Map<string, VersionConfig>` with download URLs (exe/box or SSEI), optional update URLs, and OS compatibility constraints for each supported SQL Server version (2008–2025). SQL Server 2025+ uses the SSEI bootstrapper model (`sseiUrl`) instead of direct exe/box downloads. Its media is downloaded into an isolated temporary directory before extracting and caching the installer. |
There was a problem hiding this comment.
nit: "Its" here reads as if it refers to the version registry, but the behaviour lives in downloadSseiInstaller() in src/utils.ts. Maybe something like:
| **Version registry:** `src/versions.ts` defines a `Map<string, VersionConfig>` with download URLs (exe/box or SSEI), optional update URLs, and OS compatibility constraints for each supported SQL Server version (2008–2025). SQL Server 2025+ uses the SSEI bootstrapper model (`sseiUrl`) instead of direct exe/box downloads. Its media is downloaded into an isolated temporary directory before extracting and caching the installer. | |
| **Version registry:** `src/versions.ts` defines a `Map<string, VersionConfig>` with download URLs (exe/box or SSEI), optional update URLs, and OS compatibility constraints for each supported SQL Server version (2008–2025). SQL Server 2025+ uses the SSEI bootstrapper model (`sseiUrl`) instead of direct exe/box downloads. For SSEI versions, `downloadSseiInstaller()` downloads the media into an isolated temporary directory before extracting and caching the installer. |
| await exec.exec(`"${sseiPath}"`, [ | ||
| '/Action=Download', | ||
| `/MediaPath=${mediaDir}`, | ||
| `/MediaPath="${mediaDir}"`, |
There was a problem hiding this comment.
It'd be good to quote the other paths we pass with windowsVerbatimArguments: true in the same way, so the action handles spaces consistently. These are currently unquoted:
/UpdateSource=${dirname(updatePath)}insrc/install.ts- the
.msipath after/iinsrc/installers/msi-installer.ts
| } | ||
| // use the bootstrapper to download the actual media | ||
| const mediaDir = dirname(sseiPath); | ||
| const mediaDir = await mkdtemp(joinPaths(dirname(sseiPath), 'sqlserver-media-')); |
There was a problem hiding this comment.
It'd be good to bring downloadBoxInstaller in line with this too. It still extracts into dirname(exePath) (the shared runner temp directory) and caches <temp>/setup, so the box and SSEI paths now behave differently. Giving the box extraction its own mkdtemp directory, perhaps via a small shared helper, would keep the two aligned.
Documents and improves SQL Server 2025 install support.
Changes
.exefiles in the download dir can't be picked by mistake./MediaPathso paths with spaces work.sql-2025to the CI runaction matrix.action.ymlinput and add a SQL 2025 section to README.Testing
npm run test: 92 passingnpm run lint,npm run build: cleancc @arthurschreiber @saurabh500 @David-Engel