feat(install): air-gapped / GHE env vars for install.ps1#1246
Conversation
Mirror install.sh: APM_INSTALL_DIR, GITHUB_URL, APM_REPO, VERSION; derive GH API root from GITHUB_URL; skip releases/latest when VERSION is set.
|
ping @danielmeppiel |
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 1 | 2 | Sound dual-path (pinned/non-pinned) design; Get-GitHubApiRoot correctly isolated; main risk is mutable $headers shared across three stages without explicit caching contract. |
| CLI Logging Expert | 0 | 2 | 2 | Logging is largely clean and consistent; two recommended fixes: gate the debug download-base URL behind the failure path, and surface auth-failure detail in the pinned-version download catch block. |
| DevX UX Expert | 0 | 2 | 3 | Solid Unix-parity addition; two recommended fixes: document/remove the dead $args[0] branch, and disclose the silent x86_64-on-ARM64 limitation. Nits are polish-level. |
| Supply Chain Security | 1 | 4 | 2 | Silent checksum bypass (blocking) when .sha256 is absent; recommended: enforce https on GITHUB_URL, validate APM_REPO/VERSION format, canonicalize APM_INSTALL_DIR. |
| OSS Growth Hacker | 0 | 2 | 1 | Solid Windows parity uplift that unlocks GHES/enterprise CI -- ship it, but add a windows-latest Actions snippet before release to convert the target audience. |
| Auth Expert | 0 | 1 | 1 | Pinned-path auth failure is silent; non-pinned path has good error UX -- mirror its token-absent guard into the pinned download retry block. |
| Doc Writer | 0 | 2 | 2 | Docs are accurate and well-structured; two recommended gaps in the skill doc (missing APM_INSTALL_DIR example, no cross-reference to full variable table) plus two nits. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 5 follow-ups
- [Supply Chain Security] (blocking-severity) Treat missing/undownloadable .sha256 as a hard error for pinned installs; add --skip-checksum opt-out flag. -- Silent checksum bypass collapses the only integrity gate for air-gapped/GHES pinned installs, the exact audience this PR targets. Fix is two lines in the catch block. Pre-ship blocker.
- [Auth Expert] (blocking-severity) Mirror the non-pinned path's token-absent guard and error message into the pinned download retry block. -- Private-repo users pinning VERSION on GHES get "All download attempts failed" with no hint to set GITHUB_APM_PAT. Non-pinned path already has the correct UX; parity is a one-function change. Ship in same commit as checksum fix.
- [Supply Chain Security] Enforce https:// scheme on GITHUB_URL; validate APM_REPO owner/name format; validate VERSION against semver pattern; canonicalize APM_INSTALL_DIR with GetFullPath(). -- Defense-in-depth cluster. No regression introduced by this PR, but the new env-var surface expands the attack area. Fast follow-up issue covers all four in one pass.
- [DevX UX Expert] Add one-line disclosure in docs: ARM64 Windows installs x86_64 via emulation; note air-gapped GHES users should save install.ps1 locally. -- Silent arch mismatch is a trust issue for enterprise users; one sentence in the docs table closes it cleanly.
- [OSS Growth Hacker] Add a windows-latest GitHub Actions YAML snippet (env block + irm|iex + apm install --frozen) to installation.md in this release cycle. -- Enterprise Windows CI adoption happens through Actions YAML copy-paste. This is the highest-ROI doc addition for converting the target audience unlocked by this PR.
Architecture
classDiagram
direction TB
class install_ps1 {
<<IOBoundary>>
+githubUrl string
+apmRepo string
+pinnedVersion string
+apiRoot string
+headers hashtable
+tagName string
+asset object
+binDir string
+installRoot string
+releasesDir string
}
class Get_GitHubApiRoot {
<<Pure>>
+param Url string
+returns string
}
class Get_AuthHeader {
<<Pure>>
+returns hashtable
}
class Invoke_GitHubJson {
<<NET>>
+param Uri string
+param Headers hashtable
+returns object
}
class Write_ManualInstallHelp {
<<Pure>>
+param GithubUrl string
+param ApmRepo string
}
class Add_ToUserPath {
<<FS>>
+param PathEntry string
}
class Install_ViaPip {
<<EXEC>>
+returns bool
}
class Test_PythonRequirement {
<<EXEC>>
+returns string
}
install_ps1 ..> Get_GitHubApiRoot : derives apiRoot
install_ps1 ..> Get_AuthHeader : lazy auth (3 call sites)
install_ps1 ..> Invoke_GitHubJson : auth retry only
install_ps1 ..> Write_ManualInstallHelp : on every exit-1 path
install_ps1 ..> Add_ToUserPath : post-install
install_ps1 ..> Install_ViaPip : binary fallback
Install_ViaPip ..> Test_PythonRequirement : checks python
class Get_GitHubApiRoot:::touched
class Invoke_GitHubJson:::touched
class Write_ManualInstallHelp:::touched
class install_ps1:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A["irm (aka.ms/redacted) | iex"] --> B["param: Version pos=0, Repo"]
B --> C["[FS] Resolve APM_INSTALL_DIR -> binDir / installRoot / releasesDir"]
C --> D["Get-GitHubApiRoot(githubUrl) -> apiRoot"]
D --> E{"pinnedVersion set?"}
E -->|"Yes"| F["tagName = pinnedVersion\nskip releases/latest API"]
E -->|"No"| G["[NET] Invoke-RestMethod apiRoot/repos/apmRepo/releases/latest (unauth)"]
G --> H{"release.tag_name?"}
H -->|"Yes"| I["tagName = release.tag_name\nasset = matching release asset"]
H -->|"No"| J["Get-AuthHeader -> headers\n(GITHUB_APM_PAT or GITHUB_TOKEN)"]
J --> K{"headers.Count > 0?"}
K -->|"No"| ERR1["Write-ManualInstallHelp\nexit 1"]
K -->|"Yes"| L["[NET] Invoke-GitHubJson latestUri + headers"]
L --> M{"tag resolved?"}
M -->|"No"| ERR1
M -->|"Yes"| I
F --> DL1["[NET] Invoke-WebRequest directUrl (unauth)"]
DL1 --> DLF1{"downloadOk?"}
DLF1 -->|"No"| DLA1["Get-AuthHeader; retry with token"]
DLA1 --> DLF1B{"downloadOk?"}
I --> DL2["[NET] 3-stage fallback:\nbrowser_download_url / API asset / direct+auth"]
DL2 --> DLF2{"downloadOk?"}
DLF1B -->|"No"| FALLBACK
DLF1B -->|"Yes"| CHKSUM
DLF2 -->|"No"| FALLBACK
DLF2 -->|"Yes"| CHKSUM
FALLBACK["Install-ViaPip fallback\nWrite-ManualInstallHelp / exit 1"]
CHKSUM["[NET] Fetch sha256\n(pinned: direct URL; non-pinned: asset metadata)"]
CHKSUM --> CHKF{"fetched and hash match?"}
CHKF -->|"No/mismatch"| FALLBACK
CHKF -->|"Yes"| EXTRACT
EXTRACT["[FS] Expand-Archive -> tempDir"]
EXTRACT --> BINTEST["[EXEC] apm.exe --version"]
BINTEST --> BTO{"exit 0?"}
BTO -->|"No"| FALLBACK
BTO -->|"Yes"| INSTALL
INSTALL["[FS] Copy to releases/tagName\nWrite apm.cmd shim -> binDir"]
INSTALL --> PATH["Add-ToUserPath binDir"]
PATH --> DONE["Write-Success + usage hints\nexit 0"]
Recommendation
Fix the two pre-ship items -- silent checksum bypass for pinned installs and absent token diagnostics in the pinned download retry -- before merge; both are small, scoped, and in the same code region. Once those land, this PR ships as needs_rework-resolved: the input-validation cluster, ARM64 disclosure, Actions snippet, and doc cross-reference are tracked as fast follow-ups in the same release milestone. The CHANGELOG entry should be rephrased to lead with the user outcome before release.
Full per-persona findings
Python Architect
- [recommended] Script-scope mutable $headers creates implicit shared state across Stage 1, Stage 2, and checksum verification
$headers is initialised to @{} at script scope and lazily overwritten by Get-AuthHeader at multiple points. Each site callsif ($headers.Count -eq 0) { $headers = Get-AuthHeader }independently -- correct for single-threaded execution, but implicit state bleed means a maintainer adding a fourth call site must know $headers may already be populated. An Ensure-AuthHeader helper would make the caching contract explicit and remove the repeated guard idiom (four occurrences). - [nit] Invoke-GitHubJson bypassed for the initial unauthenticated Stage-1 call
Stage 1's first attempt calls Invoke-RestMethod directly; auth retry uses Invoke-GitHubJson. Using the helper consistently keeps all REST calls in one place. - [nit] $args[0] fallback for version is silent dead code in saved-script invocation; comment missing for iex scenario
In the param-bound path, $Version captures Position=0 first. The $args branch is a shim for irm|iex piping where PowerShell binding differs, but without a comment a maintainer reads it as dead code.
CLI Logging Expert
- [recommended] Download base URL always shown for pinned installs, with no verbose gate
Write-Info "Download base: $githubUrl/$apmRepo/releases/download/$tagName/"fires unconditionally on every pinned-version install. This is debug-level info that adds noise in CI. The URL also appears in the failure diagnostic; defer it to the failure path. - [recommended] Silent catch on authenticated download failure loses error context in pinned path
In the pinned-version download branch,catch { }after the authenticated Invoke-WebRequest swallows the exception silently. Both unauthenticated and authenticated failure collapse to "All download attempts failed" with no indication of WHY authentication failed. Non-pinned path correctly emits Write-WarningText on each failure. - [nit] Two success messages ("Download successful" vs "Download successful with authentication") are indistinct at summary level
Both could read "Download successful"; the chain order already tells the story in diagnostic scenarios. - [nit] Documentation URL in success footer uses $githubUrl/$apmRepo which points to fork for GHES users, not canonical docs
Write-Host "Documentation: $githubUrl/$apmRepo"will show the GHES mirror URL, which unlikely hosts docs.
DevX UX Expert
- [recommended] $args[0] version fallback is dead code -- positional param [Position=0] $Version already consumes it
PowerShell binds the first positional argument to $Version before the script body runs. Theelseif ($args.Count -gt 0)branch can never be reached. Either remove or add a comment explaining the irm|iex piping edge case. - [recommended] ARM64 Windows installs x86_64 silently; docs and script don't disclose this
install.ps1 hardcodes "apm-windows-x86_64.zip" with no architecture detection. install.sh detects arch. Docs should include: "ARM64 Windows: the installer currently downloads the x86_64 build, which runs via emulation." - [nit] GHES example in docs uses irm (aka.ms/redacted) -- that URL is unreachable on a true air-gapped GHES network
A comment "# Air-gapped: save install.ps1 locally first, then: .\install.ps1 v1.2.3" would prevent this footgun. - [nit] TrimStart('@') accepting
@v1.2.3 is undocumented -- either document it or remove it
If intentional (matching copy-paste from package.json), a single comment makes it discoverable. - [nit] Single-segment APM_INSTALL_DIR edge case (installRoot == binDir) is undocumented
When $env:APM_INSTALL_DIR = "bin" (no parent), installRoot equals binDir. A table note ("prefer an absolute path") prevents confusion.
Supply Chain Security
- [blocking] Checksum verification silently skipped on 404/network error -- binary installed without integrity check at
install.ps1
When the .sha256 download fails for any reason, the outer catch sets $fetched = $false and falls through to extract and deploy the binary with no integrity assertion. For pinned-version installs on air-gapped GHES runners -- the primary audience this PR unlocks -- the SHA256 file download is the ONLY integrity gate; silently skipping it on the most common failure mode (file absent on release) means the guarantee collapses exactly when most relied upon. Fix: treat a missing or undownloadable checksum as a hard error when $pinnedVersion is set, unless the user explicitly opts out with a --skip-checksum flag. - [recommended] No HTTPS scheme enforcement on $env:GITHUB_URL -- allows plaintext HTTP downloads
GITHUB_URL accepted after Trim/TrimEnd only. A value like (internalmirror.corp/redacted) causes binary AND its checksum to be fetched over plaintext HTTP. Because checksum and binary are fetched from the same potentially-tampered origin, an on-path attacker can serve a matching fake hash alongside a malicious binary. Add:if ($githubUrl -notmatch '^https://') { Write-ErrorText 'GITHUB_URL must use https://'; exit 1 } - [recommended] No format validation on $env:APM_REPO -- arbitrary string interpolated into API and download URLs
APM_REPO is interpolated directly into API and download URLs without owner/name format check. A guard ($apmRepo -notmatch '^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$') closes this. - [recommended] $env:VERSION not validated against semver pattern before URL interpolation
pinnedVersion receives only TrimStart('@') before interpolation. Validate with-notmatch '^v?[0-9]+\.[0-9]+'to fail fast with a useful message. - [recommended] $env:APM_INSTALL_DIR not canonicalized -- no containment check before use in archive extraction
Path receives Trim/TrimEnd only. Should use [System.IO.Path]::GetFullPath() and validate absolute path before New-Item/Expand-Archive. - [nit] Write-Host "Direct URL was: $directUrl" leaks full constructed URL to terminal on failure
Minor information disclosure on error path; fine for debugging. - [nit] irm|iex bootstrap has no code-signing check -- TOFU on first install
Pre-existing pattern, industry-standard for Windows CLI installers. No regression introduced by this PR.
OSS Growth Hacker
- [recommended] Missing GitHub Actions windows-latest CI snippet in installation docs -- the primary adoption surface for enterprise Windows CI users
The docs show env vars for interactive PowerShell sessions but omit a runs-on: windows-latest GitHub Actions workflow block. Enterprise Windows CI adoption overwhelmingly happens through Actions YAML copy-paste, not one-liners typed at a terminal. A 10-line example (env block + irm|iex step + apm install --frozen) would be the single highest-ROI addition. - [recommended] CHANGELOG entry lacks a CI/enterprise-facing hook phrase that makes it shareable as a release beat
Growth-optimised phrasing: "Windows CI pipelines and GHES runners can now pin a VERSION and point GITHUB_URL to an internal mirror -- matching the full env-var surface of install.sh." Current entry buries the unlock after the variable list. - [nit] install.ps1 param block shows [string]$Repo alongside $env:APM_REPO -- confusing precedence
A comment "# Prefer $env:APM_REPO; -Repo kept for back-compat" removes the ambiguity without a behavior change.
Auth Expert
- [recommended] Pinned-path auth failure is silent: no actionable error message when token is absent and unauthenticated download fails
In the non-pinned path, when unauthenticated API access fails the code explicitly checks headers.Count -eq 0 and emits "Repository may be private but no authentication token found." plus remediation advice. In the pinned path, Get-AuthHeader returning an empty hashtable silently falls through to the generic "All download attempts failed" error. A private-repo user pinning VERSION on GHES gets no hint to set GITHUB_APM_PAT or GITHUB_TOKEN. Fix: add the same headers.Count -eq 0 guard in the pinned download retry block, mirroring the non-pinned path. - [nit] Get-AuthHeader silently skips GITHUB_TOKEN scoping -- document that GITHUB_TOKEN on Actions runners may lack access on GHES
For GHES targets the token required is a GHES PAT, not a github.com GITHUB_TOKEN. A comment prevents silent 401s in CI.
Doc Writer
- [recommended] Skill doc's Windows section omits the APM_INSTALL_DIR custom-directory example present in install.ps1 comments and in getting-started/installation.md
packages/apm-guide/.apm/skills/apm-usage/installation.md shows VERSION and GHES combo but silently drops APM_INSTALL_DIR. The skill doc is used by agents as a compact reference -- a missing variable creates a silent gap for agent consumers customizing install paths on Windows. - [recommended] Skill doc's Windows section has no cross-reference to getting-started/installation.md for the full variable table
A one-line "See Installation for the full variable reference." would satisfy PROSE Orchestrated Composition without adding bulk. - [nit] GITHUB_URL table description ("Base URL for downloads") undersells its role in API endpoint derivation
"Base GitHub URL (downloads and API endpoint derivation; for GHES set to the server root)" is more precise and matches install.ps1 logic. - [nit] Commented-out positional-parameter example sits inside an executable code block, creating a mixed-mode block
"# Or pass a positional parameter..." inside the runnable PowerShell block is accurate but a readability issue. A prose sentence or separate fenced block would be cleaner.
Test Coverage Expert -- inactive
No src/**/*.py files in this PR; changed files are install.ps1 (PowerShell installer), CHANGELOG.md, docs/src/content/docs/getting-started/installation.md, and packages/apm-guide/.apm/skills/apm-usage/installation.md.
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
Note
🔒 Integrity filter blocked 2 items
The following items were blocked because they don't meet the GitHub integrity level.
- #1246
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved". - feat(install): air-gapped / GHE env vars for install.ps1 #1246
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneGenerated by PR Review Panel for issue #1246 · ● 2.9M · ◷
Fixes #668
Description
Windows PowerShell installer parity with
install.sh(#660):install.ps1now supportsAPM_INSTALL_DIR,GITHUB_URL,APM_REPO, andVERSION. PinningVERSIONskips thereleases/latestHTTP API and pulls the Windows archive from{GITHUB_URL}/{APM_REPO}/releases/download/{tag}/.... For latest-release discovery, the API base is derived fromGITHUB_URL(https://api.github.comon GitHub.com,{host}/api/v3on GitHub Enterprise Server).The installation docs, apm-guide skill, and CHANGELOG describe the variables and air-gapped / GHE usage.
Type of change
Testing