Skip to content

chore(license): enforce licence headers on PowerShell files - #369

Open
siloteemu wants to merge 3 commits into
mainfrom
chore/license-header-powershell
Open

chore(license): enforce licence headers on PowerShell files#369
siloteemu wants to merge 3 commits into
mainfrom
chore/license-header-powershell

Conversation

@siloteemu

@siloteemu siloteemu commented Sep 9, 2026

Copy link
Copy Markdown

Summary

install.ps1 is the Windows installer — one of the first files anyone reads before running
anything from this project — and it shipped with no copyright or licence header.

The licence-header check did not catch that, and would not have caught any future PowerShell
file either. hawkeye (run by the license-headers CI job and by the prek hook) takes its
file list from licenserc.toml's includes, which covered only .rs, .py, .sh and
.md. PowerShell files were invisible to it, so the check was passing because it never
looked at the file, not because the file complied.

The non-obvious part: adding **/*.ps1 to includes alone would not have been enough.
hawkeye v7.0.0 has no built-in comment-style rule for the ps1 extension, so it classifies
such files as unsupported, skips them silently, and still exits 0 — surfaced only under
--fail-on-unknown, which neither the CI job nor the hook passes. The explicit [[rules]]
entry (extensions = ["ps1"], style_out = "script") is what makes the check actually
enforce anything. It is load-bearing, not redundant with includes, and the config carries a
comment saying so, so it is not pruned later as dead weight.

Changes

  • Add the missing header to install.ps1.
  • Suppress PSUseBOMForUnicodeEncodedFile for install.ps1 alone, so the file can stay
    BOM-less (see below).
  • licenserc.toml: add **/*.ps1 to includes, plus the matching [[rules]] entry that
    gives hawkeye a comment style for the extension.
  • .pre-commit-config.yaml: add powershell (a verified real identify tag) to the hook's
    types_or, so it fires on PowerShell files instead of skipping them.
  • .pre-commit-config.yaml: also add markdown, in a second commit. licenserc.toml has
    enforced headers on **/*.md all along, but types_or never listed the tag, so a
    Markdown-only commit skipped the hook entirely and a missing header surfaced only in CI —
    the same gap, one line over. The list now carries one identify tag per extension family
    in includes, with a comment recording why the two have to stay in step.

The encoding consequence (third commit). The header's © is the first non-ASCII byte
install.ps1 has ever carried, which makes PSScriptAnalyzer's PSUseBOMForUnicodeEncodedFile
fire on the file. The rule's remedy — add a UTF-8 BOM — is the wrong trade here, so the file
stays BOM-less and the rule is suppressed for it.

A BOM is incompatible with irm ... | iex, which is the install command this README, the
release notes and the nightly notes all publish. Invoke-RestMethod returns a string, not a
file, so the mark survives into it and the parser glues it onto the following token: the
leading # Copyright ... comment becomes a command named #, and the param() block is no
longer the script's first statement, so all nine installer parameters silently vanish.
Measured on the BOM'd file: first character U+FEFF, parse errors 0, ParamBlock present False. Zero parse errors is what makes it dangerous — nothing announces the breakage.

Documentation cannot fix that, either: the irm ... | iex one-liner is already published in
release notes, bookmarks and runbooks, and those copies keep pointing at main. Mojibake in a
comment is strictly less harmful than a silent functional regression, so the file keeps its
BOM-less encoding.

The suppression is deliberately the narrowest mechanism that works: a file-scoped
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseBOMForUnicodeEncodedFile', '')] on
the script's param() block, not an ExcludeRules entry in PSScriptAnalyzerSettings.psd1.
The rule therefore keeps guarding any future PowerShell file added to the repo. Verified both
ways: with the attribute the analyzer reports no findings on install.ps1, and a fresh .ps1
containing a © still trips the rule. The attribute carries a comment explaining why the file
must stay BOM-less, so the next person does not "fix" it and reintroduce the regression.

The remaining cost is cosmetic and accepted: Windows PowerShell 5.1 decodes a BOM-less file as
the ANSI codepage, so the © in that comment renders as mojibake there. The README is
unchanged by this PR.

Risk: low — a comment header and check configuration; no product code path changes. No
Gherkin scenario is added: nothing a CLI user can observe changes, so this falls under the
carve-out for purely internal changes.

Test plan

  • hawkeye check -> exit 0, 231 files, 0 changes, 0 conflicts, 0 unsupported.
  • hawkeye check --fail-on-unknown -> exit 0.
  • Negative cases, showing the check is not passing vacuously:
    • with the [[rules]] block removed, install.ps1 reverts to unsupported at exit 0
      (and exit 1 only under --fail-on-unknown);
    • with the block present but the header removed, hawkeye check reports add install.ps1
      and exits 1;
    • prek run --files README.md skips the hook without the markdown tag and runs it with
      the tag, so that addition is not vacuous either.
  • prek run --all-files -> all hooks pass, including PSScriptAnalyzer.
  • The header in install.ps1 was inserted by hawkeye format rather than hand-typed, so it
    is byte-exact against the configured header text.

Encoding and installer checks (third commit):

  • Invoke-ScriptAnalyzer -Path install.ps1 -Settings PSScriptAnalyzerSettings.psd1
    (PSScriptAnalyzer 1.25.0, the version CI pins) -> no findings, exit 0. Not vacuous: a fresh
    .ps1 carrying a © and no suppression still reports PSUseBOMForUnicodeEncodedFile, so
    the rule is only silenced for this one file.

  • First 8 bytes of install.ps1 are 23 20 43 6f 70 79 72 69 — no ef bb bf.

  • The regression is checked the way it was found: read the file's bytes, decode to a string
    and parse that string (what irm ... | iex does). Result: first character U+0023, parse
    errors 0, ParamBlock present True, param count 9, and PowerShell binds all nine
    parameters (Channel, Repo, InstallDir, DownloadBase, ArchiveExtension,
    SigningPublicKeyPath, SigningPublicKeyPem, RequireSignature, NoPathUpdate). The same
    probe against a BOM'd copy gives U+FEFF, parse errors 0, ParamBlock present False.

  • The 5.1 lint leg cannot run off Windows; that half is covered by CI rather than locally.

  • If this PR fixes a bug, searched tests/e2e-cucumber/expectations.toml for the fixed ticket ID and removed/narrowed any now-stale xfail rows. — Not applicable: this fixes no product bug and involves no xfail rows.

install.ps1 shipped without the MIT/copyright header that every other
source file in the repo carries, because the header check never looked at
PowerShell files at all: `licenserc.toml` did not include `**/*.ps1`, and
the pre-commit hook's `types_or` list omitted `powershell`.

Adding the glob alone is not enough. hawkeye has no built-in comment style
for the `ps1` extension, so a file matched by `includes` but with no
matching rule is classified `unsupported` and silently skipped while the
command still exits 0 — the check would look green without inspecting the
file. The explicit `[[rules]]` block mapping `ps1` to the `script` comment
style is what makes the file actually get checked.

With all three changes, `hawkeye check` reports 230 files, 0 changes,
0 conflicts, 0 unsupported, and `--fail-on-unknown` also passes.

Signed-off-by: Teemu Kuusisto <teemu.kuusisto@amd.com>
`licenserc.toml` has enforced headers on `**/*.md` all along, but the
pre-commit hook's `types_or` never listed `markdown`, so a commit touching
only Markdown skipped the hook entirely and the missing header surfaced
only in CI — the same gap that left PowerShell files unchecked.

Add the tag so the list covers one identify tag per extension family in
`includes`, with a comment recording why the two lists have to stay in
step.

Signed-off-by: Teemu Kuusisto <teemu.kuusisto@amd.com>
The licence header this branch adds puts a copyright sign in install.ps1, the
first non-ASCII byte the file has ever carried, which makes PSScriptAnalyzer's
PSUseBOMForUnicodeEncodedFile fire on it.

Adding the BOM the rule asks for would be a silent functional regression. The
documented install path is `irm ... | iex`, which hands the parser a string
rather than a file, so the mark survives into that string and the parser glues
it onto the first token. The param() block then stops being the script's first
statement and all nine installer parameters disappear -- with zero parse errors
to show for it. Measured on the BOM'd file: first char U+FEFF, parse errors 0,
ParamBlock present False.

So the file stays BOM-less and the rule is suppressed for it alone, via a
file-scoped SuppressMessageAttribute on the param() block rather than a global
ExcludeRules entry, so the rule keeps guarding any future PowerShell file. The
attribute carries a comment explaining why, since the obvious "fix"
reintroduces the regression. The remaining cost is cosmetic: Windows PowerShell
5.1 decodes the BOM-less file as the ANSI codepage and renders the copyright
sign in that comment as mojibake.

Signed-off-by: Teemu Kuusisto <teemu.kuusisto@amd.com>
@siloteemu
siloteemu force-pushed the chore/license-header-powershell branch from 19b9533 to 72b0311 Compare September 9, 2026 10:07
@siloteemu
siloteemu marked this pull request as ready for review September 9, 2026 10:32
@siloteemu
siloteemu requested a review from a team as a code owner September 9, 2026 10:32
@siloteemu
siloteemu requested a review from fredespi September 9, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant