Skip to content

Commit 0ded835

Browse files
authored
Merge pull request #316: fix(powershell): bind positional scan paths to -Paths
The wrapper's own documented invocation scanned the wrong tree. `--` ends PowerShell's parameter parsing, what follows binds positionally, and $Root was declared first — so `own-check.ps1 -Format msbuild -- src\MyApp` made src\MyApp the checkout root and let the scan fall through to ".". Bare positional paths did the same; only an explicit -Paths arrived. The failure was silent, which is what made it worth a slice of its own: ask for directory A, get directory B analysed, and a clean B hands back a green result with no error at all. Same shape as the lie A1 and A4 removed — "did not look" wearing the face of "looked and found nothing" — except the tool did look, just not where it was sent. Fixed by declaring the contract rather than inheriting it from declaration order: PositionalBinding = $false, and $Paths claims Position 0 explicitly. Reordering the param block would have worked today and left the meaning resting on the same implicit machinery that had already produced this twice. CI gains the half that actually catches a wrong-tree scan: from a CLEAN working directory, ask for a target holding a guaranteed OWN001 and require the finding to return while the cwd's own file never appears. The `--` and -Paths forms must produce identical output, and two positional paths must both arrive — [string[]] is the declared type, and a one-element proof does not establish it. Known boundary, documented rather than papered over: under `pwsh -File` the `--` token is intercepted by the host before parameter binding. That is PowerShell's own -File handling; the supported forms there are a bare positional path or an explicit -Paths.
2 parents 0e5f61e + bfcab5e commit 0ded835

2 files changed

Lines changed: 76 additions & 5 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2112,8 +2112,10 @@ jobs:
21122112
21132113
# The PowerShell wrapper's exit-code tiers, on the platform it exists for
21142114
# (#313). own-check.ps1 is the Windows twin of own-check.sh and is never
2115-
# exercised by any other job: the composite action drives the .sh, and the
2116-
# .ps1's paths are built with backslashes, so it cannot run on Linux at all.
2115+
# exercised by any other job. The composite action drives own-check.sh. The
2116+
# PowerShell wrapper can execute far enough on Linux to prove its stage-1
2117+
# failure tier, but its successful extraction path constructs Windows-style
2118+
# paths and is therefore exercised end to end on Windows.
21172119
# That left the stage-1 normalisation — a broken extractor must land in the
21182120
# >=2 tier, not borrow exit 1 from "findings present" — mirrored in code and
21192121
# proven nowhere. Symmetry of source is an argument, not evidence.
@@ -2157,6 +2159,21 @@ jobs:
21572159
New-Item -ItemType Directory -Force -Path $clean | Out-Null
21582160
'public class Clean { public int M() { return 1; } }' |
21592161
Set-Content -Path (Join-Path $clean "Clean.cs")
2162+
- name: Fixtures for the invocation contract (a leaky target, a clean cwd)
2163+
run: |
2164+
# The target the user will ASK for, and a working directory that has
2165+
# nothing to report — so "scanned the wrong tree" cannot hide behind
2166+
# a finding that happens to exist in both.
2167+
foreach ($n in @("ps1-target-a", "ps1-target-b")) {
2168+
$d = Join-Path $env:RUNNER_TEMP $n
2169+
New-Item -ItemType Directory -Force -Path $d | Out-Null
2170+
"using System.IO;`npublic class Leaky_$($n -replace '-','_') { public void Run() { var s = new MemoryStream(); s.WriteByte(1); } }" |
2171+
Set-Content -Path (Join-Path $d "Leaky.cs")
2172+
}
2173+
$cwd = Join-Path $env:RUNNER_TEMP "ps1-cwd-clean"
2174+
New-Item -ItemType Directory -Force -Path $cwd | Out-Null
2175+
'public class NothingHere { public int M() { return 1; } }' |
2176+
Set-Content -Path (Join-Path $cwd "NothingHere.cs")
21602177
- name: "Tier >=2 — a broken stage 1 is a hard error, never the findings tier"
21612178
run: |
21622179
# -Root at a checkout that has no extractor project: `dotnet run`
@@ -2201,6 +2218,51 @@ jobs:
22012218
}
22022219
Write-Host "OK: findings -> 1 with the flag, 0 without, identical output"
22032220
exit 0
2221+
- name: "Invocation contract — the requested target is what gets scanned (#315)"
2222+
run: |
2223+
# The defect this pins: `own-check.ps1 -Format github -- src\App` used
2224+
# to bind src\App to -Root and scan "." instead — the tool looked, but
2225+
# not where it was asked to. Standing in a CLEAN directory is what
2226+
# makes that visible: a wrong-tree scan comes back with nothing and
2227+
# reads as good news.
2228+
$script = Join-Path $env:GITHUB_WORKSPACE "scripts/own-check.ps1"
2229+
$targetA = Join-Path $env:RUNNER_TEMP "ps1-target-a"
2230+
$targetB = Join-Path $env:RUNNER_TEMP "ps1-target-b"
2231+
Push-Location (Join-Path $env:RUNNER_TEMP "ps1-cwd-clean")
2232+
try {
2233+
$sep = & $script -Format github -- $targetA
2234+
$expl = & $script -Format github -Paths $targetA
2235+
$pos = & $script -Format github $targetA
2236+
$many = & $script -Format github -- $targetA $targetB
2237+
$none = & $script -Format github
2238+
} finally { Pop-Location }
2239+
2240+
foreach ($case in @(@{n="-- <target>"; o=$sep}, @{n="-Paths <target>"; o=$expl}, @{n="bare positional"; o=$pos})) {
2241+
if (-not (($case.o -join "`n") -match "OWN001")) {
2242+
Write-Host "FAIL: $($case.n) reported no finding — the target was not scanned"; exit 1
2243+
}
2244+
if (($case.o -join "`n") -match "NothingHere") {
2245+
Write-Host "FAIL: $($case.n) scanned the working directory instead of the target"; exit 1
2246+
}
2247+
}
2248+
# The two public forms must be EQUIVALENT, not merely both non-empty.
2249+
if (($sep -join "`n").Trim() -ne ($expl -join "`n").Trim()) {
2250+
Write-Host "FAIL: '-- <target>' and '-Paths <target>' disagree"
2251+
Write-Host "--- -- form ---"; $sep | Write-Host
2252+
Write-Host "--- -Paths form ---"; $expl | Write-Host
2253+
exit 1
2254+
}
2255+
# [string[]] is the declared type, so more than one path must work.
2256+
$manyText = $many -join "`n"
2257+
if (-not ($manyText -match "ps1-target-a") -or -not ($manyText -match "ps1-target-b")) {
2258+
Write-Host "FAIL: multiple positional paths did not both reach -Paths"; $many | Write-Host; exit 1
2259+
}
2260+
# No target at all still means the working directory, which is clean.
2261+
if (($none -join "`n") -match "OWN001") {
2262+
Write-Host "FAIL: with no target the clean cwd should report nothing"; $none | Write-Host; exit 1
2263+
}
2264+
Write-Host "OK: every documented form scans the requested target; the two public forms agree"
2265+
exit 0
22042266
22052267
# Dog-food the code-scanning surface end-to-end (P-013): run the composite action
22062268
# with format: sarif over the sample tree, then upload the log to GitHub code

‎scripts/own-check.ps1‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
.EXAMPLE
4545
scripts\own-check.ps1 -Format github -Severity warning -FailOnFinding -- .
4646
#>
47-
[CmdletBinding()]
47+
[CmdletBinding(PositionalBinding = $false)]
4848
param(
4949
[string]$Root,
5050
[string]$Format = "human",
@@ -53,7 +53,13 @@ param(
5353
[string]$Verbosity = "normal",
5454
[switch]$Legacy,
5555
[switch]$FailOnFinding,
56-
[Parameter(ValueFromRemainingArguments = $true)]
56+
# Position 0 is claimed EXPLICITLY, and automatic positional binding is off
57+
# for everything else (PositionalBinding = $false). Without both halves the
58+
# scan target lands in $Root — the first declared parameter took position 0
59+
# — and own-check then hunts for the extractor inside the tree it was asked
60+
# to scan, or silently scans "." instead. Declaring the contract beats
61+
# relying on declaration order to keep meaning it.
62+
[Parameter(Position = 0, ValueFromRemainingArguments = $true)]
5763
[string[]]$Paths
5864
)
5965

@@ -64,7 +70,10 @@ $ErrorActionPreference = "Stop"
6470
if ([string]::IsNullOrEmpty($Root)) {
6571
$Root = Split-Path -Parent $PSScriptRoot
6672
}
67-
# A bare "--" separator (shell habit) is harmless; drop it.
73+
# A bare "--" separator (shell habit) is harmless; drop it. In an interactive
74+
# session PowerShell eats the token itself, so this is a no-op there — it earns
75+
# its keep when the arguments are splatted (`& own-check.ps1 @args`), where a
76+
# literal "--" does arrive as a value.
6877
if ($Paths) { $Paths = @($Paths | Where-Object { $_ -ne "--" }) }
6978
if (-not $Paths -or $Paths.Count -eq 0) { $Paths = @(".") }
7079

0 commit comments

Comments
 (0)