Skip to content

Commit cdfd9ae

Browse files
MariusStorhaugMariusStorhaug
committed
Harden TestData parsing per review
1 parent 39864d1 commit cdfd9ae

4 files changed

Lines changed: 213 additions & 29 deletions

File tree

‎.github/workflows/AfterAll-ModuleLocal.yml‎

Lines changed: 67 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,68 @@ jobs:
4343
try {
4444
$data = $env:PSMODULE_TEST_DATA | ConvertFrom-Json -ErrorAction Stop
4545
} catch {
46-
throw "The 'TestData' secret must be a JSON object with 'secrets' and/or 'variables' maps. $_"
46+
throw "The 'TestData' secret must be valid JSON with 'secrets' and/or 'variables' maps."
47+
}
48+
if ($null -eq $data -or $data -isnot [pscustomobject]) {
49+
throw "The 'TestData' secret must be a JSON object with 'secrets' and/or 'variables' maps."
50+
}
51+
$allowedTopLevelKeys = @('secrets', 'variables')
52+
foreach ($propertyName in $data.PSObject.Properties.Name) {
53+
if ($allowedTopLevelKeys -notcontains $propertyName) {
54+
throw "The 'TestData' secret only supports 'secrets' and 'variables' maps."
55+
}
56+
}
57+
$reservedNames = @('CI', 'HOME', 'PATH', 'PWD', 'SHELL', 'PSMODULE_TEST_DATA')
58+
$reservedPrefixes = @('GITHUB_', 'RUNNER_', 'ACTIONS_')
59+
function Assert-EnvironmentName {
60+
param([string] $Name)
61+
if ($Name -notmatch '^[A-Za-z_][A-Za-z0-9_]*$') {
62+
throw "TestData keys must be valid environment variable names."
63+
}
64+
$normalized = $Name.ToUpperInvariant()
65+
if ($reservedNames -contains $normalized) {
66+
throw "TestData keys must not override reserved environment variables."
67+
}
68+
foreach ($prefix in $reservedPrefixes) {
69+
if ($normalized.StartsWith($prefix)) {
70+
throw "TestData keys must not override reserved environment variables."
71+
}
72+
}
73+
}
74+
function Assert-Map {
75+
param(
76+
[object] $Map,
77+
[string] $Name
78+
)
79+
if ($null -eq $Map) { return }
80+
if ($Map -isnot [pscustomobject]) {
81+
throw "The 'TestData.$Name' value must be a JSON object."
82+
}
83+
}
84+
function Get-EnvironmentValue {
85+
param(
86+
[object] $Value,
87+
[string] $Name
88+
)
89+
if ($null -eq $Value) { return '' }
90+
if ($Value -is [pscustomobject] -or ($Value -is [System.Collections.IEnumerable] -and $Value -isnot [string])) {
91+
throw "Values in 'TestData.$Name' must be scalar values."
92+
}
93+
return [string]$Value
4794
}
4895
function Add-EnvFromMap {
4996
param(
5097
[object] $Map,
98+
[string] $Name,
5199
[switch] $Mask
52100
)
53-
if (-not $Map) { return }
101+
Assert-Map -Map $Map -Name $Name
102+
if ($null -eq $Map) { return }
103+
$count = 0
54104
foreach ($item in $Map.PSObject.Properties) {
55105
$name = $item.Name
56-
$value = [string]$item.Value
106+
Assert-EnvironmentName -Name $name
107+
$value = Get-EnvironmentValue -Value $item.Value -Name $Name
57108
if ($Mask) {
58109
foreach ($line in ($value -split "`n")) {
59110
$line = $line.TrimEnd("`r")
@@ -62,16 +113,25 @@ jobs:
62113
}
63114
}
64115
}
65-
$delimiter = "GHENV_$([guid]::NewGuid().ToString('N'))"
116+
do {
117+
$delimiter = "GHENV_$([guid]::NewGuid().ToString('N'))"
118+
} while ($value.Contains($delimiter))
66119
Add-Content -Path $env:GITHUB_ENV -Value "$name<<$delimiter"
67120
Add-Content -Path $env:GITHUB_ENV -Value $value
68121
Add-Content -Path $env:GITHUB_ENV -Value $delimiter
69-
Write-Host "Exposed [$name] as an environment variable."
122+
$count++
123+
}
124+
if ($count -gt 0) {
125+
if ($Mask) {
126+
Write-Host "Exposed $count secret value(s) as environment variables."
127+
} else {
128+
Write-Host "Exposed $count variable value(s) as environment variables."
129+
}
70130
}
71131
}
72132
73-
Add-EnvFromMap -Map $data.secrets -Mask
74-
Add-EnvFromMap -Map $data.variables
133+
Add-EnvFromMap -Map $data.secrets -Name 'secrets' -Mask
134+
Add-EnvFromMap -Map $data.variables -Name 'variables'
75135
76136
- name: Run AfterAll Teardown Scripts
77137
if: always()

‎.github/workflows/BeforeAll-ModuleLocal.yml‎

Lines changed: 67 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,17 +43,68 @@ jobs:
4343
try {
4444
$data = $env:PSMODULE_TEST_DATA | ConvertFrom-Json -ErrorAction Stop
4545
} catch {
46-
throw "The 'TestData' secret must be a JSON object with 'secrets' and/or 'variables' maps. $_"
46+
throw "The 'TestData' secret must be valid JSON with 'secrets' and/or 'variables' maps."
47+
}
48+
if ($null -eq $data -or $data -isnot [pscustomobject]) {
49+
throw "The 'TestData' secret must be a JSON object with 'secrets' and/or 'variables' maps."
50+
}
51+
$allowedTopLevelKeys = @('secrets', 'variables')
52+
foreach ($propertyName in $data.PSObject.Properties.Name) {
53+
if ($allowedTopLevelKeys -notcontains $propertyName) {
54+
throw "The 'TestData' secret only supports 'secrets' and 'variables' maps."
55+
}
56+
}
57+
$reservedNames = @('CI', 'HOME', 'PATH', 'PWD', 'SHELL', 'PSMODULE_TEST_DATA')
58+
$reservedPrefixes = @('GITHUB_', 'RUNNER_', 'ACTIONS_')
59+
function Assert-EnvironmentName {
60+
param([string] $Name)
61+
if ($Name -notmatch '^[A-Za-z_][A-Za-z0-9_]*$') {
62+
throw "TestData keys must be valid environment variable names."
63+
}
64+
$normalized = $Name.ToUpperInvariant()
65+
if ($reservedNames -contains $normalized) {
66+
throw "TestData keys must not override reserved environment variables."
67+
}
68+
foreach ($prefix in $reservedPrefixes) {
69+
if ($normalized.StartsWith($prefix)) {
70+
throw "TestData keys must not override reserved environment variables."
71+
}
72+
}
73+
}
74+
function Assert-Map {
75+
param(
76+
[object] $Map,
77+
[string] $Name
78+
)
79+
if ($null -eq $Map) { return }
80+
if ($Map -isnot [pscustomobject]) {
81+
throw "The 'TestData.$Name' value must be a JSON object."
82+
}
83+
}
84+
function Get-EnvironmentValue {
85+
param(
86+
[object] $Value,
87+
[string] $Name
88+
)
89+
if ($null -eq $Value) { return '' }
90+
if ($Value -is [pscustomobject] -or ($Value -is [System.Collections.IEnumerable] -and $Value -isnot [string])) {
91+
throw "Values in 'TestData.$Name' must be scalar values."
92+
}
93+
return [string]$Value
4794
}
4895
function Add-EnvFromMap {
4996
param(
5097
[object] $Map,
98+
[string] $Name,
5199
[switch] $Mask
52100
)
53-
if (-not $Map) { return }
101+
Assert-Map -Map $Map -Name $Name
102+
if ($null -eq $Map) { return }
103+
$count = 0
54104
foreach ($item in $Map.PSObject.Properties) {
55105
$name = $item.Name
56-
$value = [string]$item.Value
106+
Assert-EnvironmentName -Name $name
107+
$value = Get-EnvironmentValue -Value $item.Value -Name $Name
57108
if ($Mask) {
58109
foreach ($line in ($value -split "`n")) {
59110
$line = $line.TrimEnd("`r")
@@ -62,16 +113,25 @@ jobs:
62113
}
63114
}
64115
}
65-
$delimiter = "GHENV_$([guid]::NewGuid().ToString('N'))"
116+
do {
117+
$delimiter = "GHENV_$([guid]::NewGuid().ToString('N'))"
118+
} while ($value.Contains($delimiter))
66119
Add-Content -Path $env:GITHUB_ENV -Value "$name<<$delimiter"
67120
Add-Content -Path $env:GITHUB_ENV -Value $value
68121
Add-Content -Path $env:GITHUB_ENV -Value $delimiter
69-
Write-Host "Exposed [$name] as an environment variable."
122+
$count++
123+
}
124+
if ($count -gt 0) {
125+
if ($Mask) {
126+
Write-Host "Exposed $count secret value(s) as environment variables."
127+
} else {
128+
Write-Host "Exposed $count variable value(s) as environment variables."
129+
}
70130
}
71131
}
72132
73-
Add-EnvFromMap -Map $data.secrets -Mask
74-
Add-EnvFromMap -Map $data.variables
133+
Add-EnvFromMap -Map $data.secrets -Name 'secrets' -Mask
134+
Add-EnvFromMap -Map $data.variables -Name 'variables'
75135
76136
- name: Run BeforeAll Setup Scripts
77137
uses: PSModule/GitHub-Script@1ee97bbc652d19c38ae12f6e1e47e9d9fbd12d0a # v1.8.0

‎.github/workflows/Test-ModuleLocal.yml‎

Lines changed: 67 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,68 @@ jobs:
4848
try {
4949
$data = $env:PSMODULE_TEST_DATA | ConvertFrom-Json -ErrorAction Stop
5050
} catch {
51-
throw "The 'TestData' secret must be a JSON object with 'secrets' and/or 'variables' maps. $_"
51+
throw "The 'TestData' secret must be valid JSON with 'secrets' and/or 'variables' maps."
52+
}
53+
if ($null -eq $data -or $data -isnot [pscustomobject]) {
54+
throw "The 'TestData' secret must be a JSON object with 'secrets' and/or 'variables' maps."
55+
}
56+
$allowedTopLevelKeys = @('secrets', 'variables')
57+
foreach ($propertyName in $data.PSObject.Properties.Name) {
58+
if ($allowedTopLevelKeys -notcontains $propertyName) {
59+
throw "The 'TestData' secret only supports 'secrets' and 'variables' maps."
60+
}
61+
}
62+
$reservedNames = @('CI', 'HOME', 'PATH', 'PWD', 'SHELL', 'PSMODULE_TEST_DATA')
63+
$reservedPrefixes = @('GITHUB_', 'RUNNER_', 'ACTIONS_')
64+
function Assert-EnvironmentName {
65+
param([string] $Name)
66+
if ($Name -notmatch '^[A-Za-z_][A-Za-z0-9_]*$') {
67+
throw "TestData keys must be valid environment variable names."
68+
}
69+
$normalized = $Name.ToUpperInvariant()
70+
if ($reservedNames -contains $normalized) {
71+
throw "TestData keys must not override reserved environment variables."
72+
}
73+
foreach ($prefix in $reservedPrefixes) {
74+
if ($normalized.StartsWith($prefix)) {
75+
throw "TestData keys must not override reserved environment variables."
76+
}
77+
}
78+
}
79+
function Assert-Map {
80+
param(
81+
[object] $Map,
82+
[string] $Name
83+
)
84+
if ($null -eq $Map) { return }
85+
if ($Map -isnot [pscustomobject]) {
86+
throw "The 'TestData.$Name' value must be a JSON object."
87+
}
88+
}
89+
function Get-EnvironmentValue {
90+
param(
91+
[object] $Value,
92+
[string] $Name
93+
)
94+
if ($null -eq $Value) { return '' }
95+
if ($Value -is [pscustomobject] -or ($Value -is [System.Collections.IEnumerable] -and $Value -isnot [string])) {
96+
throw "Values in 'TestData.$Name' must be scalar values."
97+
}
98+
return [string]$Value
5299
}
53100
function Add-EnvFromMap {
54101
param(
55102
[object] $Map,
103+
[string] $Name,
56104
[switch] $Mask
57105
)
58-
if (-not $Map) { return }
106+
Assert-Map -Map $Map -Name $Name
107+
if ($null -eq $Map) { return }
108+
$count = 0
59109
foreach ($item in $Map.PSObject.Properties) {
60110
$name = $item.Name
61-
$value = [string]$item.Value
111+
Assert-EnvironmentName -Name $name
112+
$value = Get-EnvironmentValue -Value $item.Value -Name $Name
62113
if ($Mask) {
63114
foreach ($line in ($value -split "`n")) {
64115
$line = $line.TrimEnd("`r")
@@ -67,16 +118,25 @@ jobs:
67118
}
68119
}
69120
}
70-
$delimiter = "GHENV_$([guid]::NewGuid().ToString('N'))"
121+
do {
122+
$delimiter = "GHENV_$([guid]::NewGuid().ToString('N'))"
123+
} while ($value.Contains($delimiter))
71124
Add-Content -Path $env:GITHUB_ENV -Value "$name<<$delimiter"
72125
Add-Content -Path $env:GITHUB_ENV -Value $value
73126
Add-Content -Path $env:GITHUB_ENV -Value $delimiter
74-
Write-Host "Exposed [$name] as an environment variable."
127+
$count++
128+
}
129+
if ($count -gt 0) {
130+
if ($Mask) {
131+
Write-Host "Exposed $count secret value(s) as environment variables."
132+
} else {
133+
Write-Host "Exposed $count variable value(s) as environment variables."
134+
}
75135
}
76136
}
77137
78-
Add-EnvFromMap -Map $data.secrets -Mask
79-
Add-EnvFromMap -Map $data.variables
138+
Add-EnvFromMap -Map $data.secrets -Name 'secrets' -Mask
139+
Add-EnvFromMap -Map $data.variables -Name 'variables'
80140
81141
- name: Download module artifact
82142
uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1

‎README.md‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ jobs:
378378
Process-PSModule:
379379
uses: PSModule/Process-PSModule/.github/workflows/workflow.yml@v5
380380
secrets:
381-
APIKEY: ${{ secrets.APIKEY }}
381+
APIKey: ${{ secrets.APIKey }}
382382
```
383383
384384
</details>
@@ -397,8 +397,9 @@ jobs:
397397

398398
### Secrets
399399

400-
The workflow declares only two secrets, which keeps the calling workflow in full control of the
401-
credentials that are exposed. `secrets: inherit` is intentionally not required.
400+
The reusable workflow at `.github/workflows/workflow.yml` declares only two workflow-call secrets,
401+
which keeps the calling workflow in full control of the credentials that are exposed.
402+
`secrets: inherit` is intentionally not required.
402403

403404
| Name | Location | Description | Required |
404405
| ---- | -------- | ----------- | -------- |
@@ -426,7 +427,7 @@ jobs:
426427
Process-PSModule:
427428
uses: PSModule/Process-PSModule/.github/workflows/workflow.yml@v5
428429
secrets:
429-
APIKey: ${{ secrets.APIKEY }}
430+
APIKey: ${{ secrets.APIKey }}
430431
TestData: >-
431432
{ "secrets": { "TEST_USER_PAT": "${{ secrets.TEST_USER_PAT }}",
432433
"TEST_APP_ORG_CLIENT_ID": "${{ secrets.TEST_APP_ORG_CLIENT_ID }}" } }
@@ -453,7 +454,7 @@ jobs:
453454
Process-PSModule:
454455
uses: PSModule/Process-PSModule/.github/workflows/workflow.yml@v5
455456
secrets:
456-
APIKey: ${{ secrets.APIKEY }}
457+
APIKey: ${{ secrets.APIKey }}
457458
TestData: >-
458459
{ "secrets": { "CONFLUENCE_API_TOKEN": "${{ secrets.CONFLUENCE_API_TOKEN }}" },
459460
"variables": { "CONFLUENCE_SITE": ${{ toJSON(vars.CONFLUENCE_SITE) }},
@@ -471,7 +472,9 @@ $env:CONFLUENCE_SITE # from the "variables" map (not masked)
471472

472473
Notes:
473474

474-
- The names are entirely caller-defined; no secret or variable names are hard-coded in the shared workflow.
475+
- The names are caller-defined; no secret or variable names are hard-coded in the shared workflow.
476+
Names must match `^[A-Za-z_][A-Za-z0-9_]*$` and must not override reserved variables such as `PATH`,
477+
`CI`, `GITHUB_*`, `RUNNER_*` or `ACTIONS_*`.
475478
- Reference secrets as `"${{ secrets.<name> }}"` (quoted, directly) rather than
476479
`toJSON(secrets.<name>)`. The direct form keeps CodeQL's *excessive secrets exposure* check happy and
477480
works for single-line secret values. It cannot carry values that contain `"`, `\` or newlines, so
@@ -491,8 +494,9 @@ Notes:
491494
- Omit `TestData` entirely when the module needs no secrets or variables. Include only the map you
492495
need (just `secrets`, just `variables`, or both).
493496
- Because `secrets: inherit` is not used, only the values you list are ever exposed.
494-
- Organization and repository secrets and variables are supported. Secrets stored in a GitHub
495-
*Environment* are not exposed by this mechanism.
497+
- Organization, repository and GitHub *Environment* secrets and variables are supported when they are
498+
visible to the calling job. For environment-scoped values, set `environment:` on the calling job and
499+
explicitly include those values in `TestData`; they are not exposed automatically.
496500

497501
### Permissions
498502

0 commit comments

Comments
 (0)