Repository navigation
fix: harden SQL Server 2025 installer downloads - #296
Conversation
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.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Box and SSEI installers now share a helper that extracts into a separate temp directory before caching. Quote /UpdateSource and the MSI path so paths with spaces work. Apply review docs wording. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
308f732 to
0b1a8dd
Compare
|
Thanks for your work on these two PRs :) |
|
🎉 This PR is included in version 4.0.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
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