Skip to content

fix(installer): Windows one-client guard reads clientId via ConvertFrom-Json (#200) #340

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LukasWodka wants to merge 1 commit into
base: develop
Choose a base branch
from
+89 −5
Open

fix(installer): Windows one-client guard reads clientId via ConvertFrom-Json (#200) #340

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions scripts/install-k8s.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1167,14 +1167,24 @@ function Install-ClientHelm {
# Check ANY namespace: a fresh install lands in 'tracebloc', but an install
# from an older installer version may be in a different namespace. Enumerate
# client-chart releases and read each clientId (ConvertFrom-Json -- no jq).
# Values are read with `-o json`, not as YAML: helm re-serializes values on
# `get`, so the YAML view quotes clientId inconsistently (typically not at
# all) and a quote-expecting regex silently bypassed this guard (#200).
$existingId = ""; $existingNs = ""
$listJson = (helm list -A -o json 2>$null) | Out-String
if ($LASTEXITCODE -eq 0 -and $listJson.Trim()) {
try {
foreach ($rel in ($listJson | ConvertFrom-Json)) {
if ($rel.chart -and $rel.chart.StartsWith("client-")) {
$vals = (helm get values $rel.name -n $rel.namespace 2>$null) | Out-String
if ($vals -match 'clientId:\s*"([^"]+)"') { $existingId = $Matches[1].Trim(); $existingNs = $rel.namespace; break }
$valsJson = (helm get values $rel.name -n $rel.namespace -o json 2>$null) | Out-String
if ($LASTEXITCODE -ne 0 -or -not $valsJson.Trim()) { continue }
# No user values serializes as literal `null` (-> $vals = $null); an
# unparsable release must not abort the scan of the remaining ones.
$vals = $null
try { $vals = $valsJson | ConvertFrom-Json } catch { continue }
if ($null -eq $vals -or $null -eq $vals.clientId) { continue }
$id = "$($vals.clientId)".Trim()
if ($id) { $existingId = $id; $existingNs = $rel.namespace; break }
}
}
} catch { }
Expand Down
2 changes: 1 addition & 1 deletion scripts/manifest.sha256
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ f890cc3917b09b4b779fcd1a53b902370cb291488a7905f9d710daa871a9eb72 scripts/lib/in
e983076d72485d58fa2c9293c62905f1f8f0f4ebd355f8f745a49f0f3bf0052c scripts/lib/provision.sh
7c5ca5fc1b3f3d5b2b699811da603b40b8879b317b1c756b8ed15da104192828 scripts/lib/summary.sh
f22b3f57722feaf3a05a8527988c4404c27b772dd8ba8d6d4c6fe08077081a4a scripts/lib/diagnose.sh
8c8973117018dee6518df1b0067d681feeee27e10e42be84e52479725b9ffe5a scripts/install-k8s.ps1
f7e48110b0c22a300d40366ecdc861b89dfc9561a366451fc2b8cf94edfd81c8 scripts/install-k8s.ps1
78 changes: 76 additions & 2 deletions scripts/tests/install-k8s.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,11 @@ Describe "Install-ClientHelm" {
Install-ClientHelm
(Get-Content "$HOST_DATA_DIR/values.yaml" -Raw) | Should -Match 'clientId: "previd"'
}
# One-client guard mocks mirror real helm (#200): `helm get values` re-serializes
# the stored values, so its YAML view typically emits clientId UNQUOTED — only
# the `-o json` view is quoting-proof, and that is what the guard must read.
# Each mock serves JSON when asked for it and the YAML view otherwise, so a
# regression back to YAML-regex-scraping fails these tests.
It "blocks a DIFFERENT client already installed" {
$HOST_DATA_DIR = "$TestDrive/d5"
Mock Err { throw "err" }
Expand All @@ -325,12 +330,78 @@ Describe "Install-ClientHelm" {
Mock Test-Credentials { "valid" }
Mock helm {
if ($args -contains "list") { '[{"name":"oldrel","namespace":"default","chart":"client-1.4.3"}]'; $global:LASTEXITCODE = 0; return }
if ($args -contains "get") { 'clientId: "otherclient"'; $global:LASTEXITCODE = 0; return }
if ($args -contains "get") {
if ($args -contains "json") { '{"clientId":"otherclient"}' } else { 'clientId: otherclient' }
$global:LASTEXITCODE = 0; return
}
$global:LASTEXITCODE = 0
}
{ Install-ClientHelm } | Should -Throw
Should -Not -Invoke helm -ParameterFilter { $args -contains "upgrade" }
}
It "blocks a DIFFERENT client whose YAML view is <style> (#200)" -TestCases @(
@{ style = 'unquoted'; yaml = 'clientId: otherclient' }
@{ style = 'single-quoted'; yaml = "clientId: 'otherclient'" }
@{ style = 'double-quoted'; yaml = 'clientId: "otherclient"' }
) {
param($style, $yaml)
$HOST_DATA_DIR = "$TestDrive/d5-$style"
$script:yamlView = $yaml
Mock Err { throw "err" }
Mock Read-Host {
param([string]$Prompt, [switch]$AsSecureString)
if ($Prompt -match 'password') { return (ConvertTo-SecureString "pw" -AsPlainText -Force) }
return "newclient"
}
Mock Test-Credentials { "valid" }
Mock helm {
if ($args -contains "list") { '[{"name":"oldrel","namespace":"default","chart":"client-1.4.3"}]'; $global:LASTEXITCODE = 0; return }
if ($args -contains "get") {
if ($args -contains "json") { '{"clientId":"otherclient"}' } else { $script:yamlView }
$global:LASTEXITCODE = 0; return
}
$global:LASTEXITCODE = 0
}
{ Install-ClientHelm } | Should -Throw
Should -Not -Invoke helm -ParameterFilter { $args -contains "upgrade" }
}
It "scans past a release with no user values and still finds the client" {
$HOST_DATA_DIR = "$TestDrive/d5-null"
Mock Err { throw "err" }
Mock Read-Host {
param([string]$Prompt, [switch]$AsSecureString)
if ($Prompt -match 'password') { return (ConvertTo-SecureString "pw" -AsPlainText -Force) }
return "newclient"
}
Mock Test-Credentials { "valid" }
Mock helm {
if ($args -contains "list") { '[{"name":"bare","namespace":"ns1","chart":"client-1.4.2"},{"name":"oldrel","namespace":"ns2","chart":"client-1.4.3"}]'; $global:LASTEXITCODE = 0; return }
if ($args -contains "get") {
# `helm get values -o json` prints literal null when nothing was set.
if ($args -contains "bare") { 'null' } else { '{"clientId":"otherclient"}' }
$global:LASTEXITCODE = 0; return
}
$global:LASTEXITCODE = 0
}
{ Install-ClientHelm } | Should -Throw
Should -Not -Invoke helm -ParameterFilter { $args -contains "upgrade" }
}
It "values without a clientId key do not trip the guard" {
$HOST_DATA_DIR = "$TestDrive/d5-nokey"
Mock Read-Host {
param([string]$Prompt, [switch]$AsSecureString)
if ($Prompt -match 'password') { return (ConvertTo-SecureString "pw" -AsPlainText -Force) }
return "newclient"
}
Mock Test-Credentials { "valid" }
Mock helm {
if ($args -contains "list") { '[{"name":"oldrel","namespace":"default","chart":"client-1.4.3"}]'; $global:LASTEXITCODE = 0; return }
if ($args -contains "get") { '{"env":{"CLIENT_ENV":"dev"}}'; $global:LASTEXITCODE = 0; return }
$global:LASTEXITCODE = 0
}
Install-ClientHelm
Should -Invoke helm -ParameterFilter { $args -contains "upgrade" }
}
It "same client re-run is allowed (upgrade in place)" {
$HOST_DATA_DIR = "$TestDrive/d6"
Mock Read-Host {
Expand All @@ -341,7 +412,10 @@ Describe "Install-ClientHelm" {
Mock Test-Credentials { "valid" }
Mock helm {
if ($args -contains "list") { '[{"name":"tracebloc","namespace":"tracebloc","chart":"client-1.4.3"}]'; $global:LASTEXITCODE = 0; return }
if ($args -contains "get") { 'clientId: "sameid"'; $global:LASTEXITCODE = 0; return }
if ($args -contains "get") {
if ($args -contains "json") { '{"clientId":"sameid"}' } else { 'clientId: sameid' }
$global:LASTEXITCODE = 0; return
}
$global:LASTEXITCODE = 0
}
Install-ClientHelm
Expand Down
Loading