diff --git a/src/docs/GitHub-Actions/Standards.md b/src/docs/GitHub-Actions/Standards.md new file mode 100644 index 0000000..f1cf4d3 --- /dev/null +++ b/src/docs/GitHub-Actions/Standards.md @@ -0,0 +1,232 @@ +# GitHub Actions Standard + +Standards for authoring and reviewing GitHub Actions workflows (`.github/workflows/*.yml`), composite actions (`action.yml`), and reusable workflows. + +> Applies whenever creating, modifying, or auditing workflow files. The [Builder](https://github.com/PSModule/.github-private/blob/main/agents/builder.md) and [Reviewer](https://github.com/PSModule/.github-private/blob/main/agents/reviewer.md) agents enforce these rules. + +## Scope + +- ✅ `.github/workflows/*.{yml,yaml}`, `action.yml`, `.github/dependabot.yml`, `CODEOWNERS` entries for workflows. +- ❌ Application source code stays out. If a fix requires changes outside `.github/`, surface it as a recommendation and stop. + +## Operating protocol + +Before writing or reviewing: + +1. **Read existing workflows.** Match shell choice, naming, job structure, comment style, conventions. +2. **Check `.github/dependabot.yml`** for `package-ecosystem: github-actions`. Propose adding it if absent. +3. **Check `CODEOWNERS`** for `.github/workflows/` ownership. Recommend if absent (don't edit `CODEOWNERS` unless explicitly asked). +4. **Resolve SHAs via `gh api`.** Never invent or guess a commit SHA. + +## Style + +### S1 — Always name jobs and steps + +Every job and every step must have a `name:` field. Names should be short, human-readable, and describe what the job or step does — not how. + +```yaml +# ✅ Required +jobs: + build: + name: Build module + steps: + - name: Checkout repository + uses: actions/checkout@ # vX.Y.Z + +# ❌ Anonymous — hard to read in the GitHub UI and logs +jobs: + build: + steps: + - uses: actions/checkout@ # vX.Y.Z +``` + +### S2 — Use quotes sparingly + +Only quote scalar values when YAML would misinterpret them without quotes (e.g. values starting with `{`, containing `:`, boolean-like strings such as `true`/`false`, numeric strings). Omit quotes everywhere else. + +```yaml +# ✅ Quotes only where needed +run-name: Release ${{ github.ref_name }} +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + +# ❌ Unnecessary quotes +run-name: 'Release ${{ github.ref_name }}' +concurrency: + group: '${{ github.workflow }}-${{ github.ref }}' +``` + +## Security + +### R1 — Pin every `uses:` to a full 40-char SHA with a patch level version comment + +```yaml +# ✅ Required +- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + +# ❌ Mutable tag +- uses: actions/checkout@v4 + +# ❌ Branch ref +- uses: actions/checkout@main +``` + +- Comment format: `# vX.Y.Z` (patch-level, `v` prefix) — Dependabot updates SHA and comment together. +- Applies to **reusable workflows** too: `uses: org/repo/.github/workflows/foo.yml@`. +- **Exception:** first-party actions in the same repo (`uses: ./.github/actions/...`). + +Resolving SHAs: + +```bash +gh api repos///git/refs/tags/ --jq '.object.sha' +# If .object.type == "tag", dereference: +gh api repos///git/tags/ --jq '.object.sha' +``` + +Always use the **commit** SHA, never the annotated-tag object SHA. + +### R2 — Declare minimum `permissions:` explicitly + +- Workflow-level `permissions:` set to the strictest needed. Default `contents: read`. +- Override per-job when one job needs more. +- **Never** `permissions: write-all` or omit `permissions:` entirely. +- Inline comment justifying any non-`read` grant (Zizmor `undocumented-permissions`). + +```yaml +permissions: + contents: read + +jobs: + deploy: + permissions: + contents: read + id-token: write # OIDC federation to AWS +``` + +### R3 — Cloud auth uses OIDC, never long-lived keys + +```yaml +permissions: + id-token: write + contents: read + +steps: + - name: Configure AWS credentials + uses: aws-actions/configure-aws-credentials@ # vX.Y.Z + with: + aws-region: ${{ vars.AWS_REGION }} + role-to-assume: ${{ vars.AWS_ROLE_ARN_CONTINUOUS_DEPLOYMENT }} +``` + +Same pattern for GCP (`google-github-actions/auth` with `workload_identity_provider`), Azure (`azure/login` with `client-id`/`tenant-id`/`subscription-id`), Vault. + +### R4 — `vars.*` for config, `secrets.*` for credentials + +- Region, account ID, role ARN, environment name → `vars.*`. +- API tokens, passwords, signing keys → `secrets.*`. +- Never hardcode account IDs, role ARNs, or region names. + +### R5 — Never interpolate untrusted context into shell + +Untrusted contexts: `github.event.issue.title`, `.body`, `.comment.body`, `.pull_request.title`, `.pull_request.body`, `.pull_request.head.ref`, `.head_commit.message`, `.review.body`, `.review_comment.body`, `.head_ref`. + +```yaml +# ❌ Template injection +- run: echo "Title: ${{ github.event.issue.title }}" + +# ✅ Pass via env var, quote in shell +- run: echo "Title: $TITLE" + env: + TITLE: ${{ github.event.issue.title }} +``` + +Zizmor `template-injection` — the #1 real-world Actions CVE pattern. + +### R6 — Avoid `pull_request_target` and `workflow_run` unless required + +- `pull_request_target` runs with **write tokens and secrets** in the base repo context while potentially checking out attacker-controlled code from a fork. If unavoidable: never `actions/checkout` the PR head, or check out into a sandbox without secrets. +- `workflow_run` has the same hazard class. +- Default to `pull_request` for PR validation; use environments + required reviewers when secrets are needed. + +### R7 — `actions/checkout` with `persist-credentials: false` unless pushing + +```yaml +- uses: actions/checkout@ # vX.Y.Z + with: + persist-credentials: false +``` + +Without this, the `GITHUB_TOKEN` is left in `.git/config` and can leak into uploaded artifacts (Zizmor `artipacked`). + +### R8 — `concurrency:` on deploy/release workflows + +```yaml +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: false # true for CI; false for deploys +``` + +### R9 — Pin `runs-on:` to a specific OS version + +```yaml +# ✅ +runs-on: ubuntu-24.04 + +# ⚠️ Acceptable but less reproducible +runs-on: ubuntu-latest +``` + +### R10 — Reusable workflows: explicit `secrets:`, never `secrets: inherit` + +```yaml +# ❌ +jobs: + call: + uses: ./.github/workflows/reusable.yml + secrets: inherit + +# ✅ +jobs: + call: + uses: ./.github/workflows/reusable.yml + secrets: + NPM_TOKEN: ${{ secrets.NPM_TOKEN }} +``` + +## Zizmor audit checklist + +Run mentally (and via [zizmor](https://github.com/woodruffw/zizmor) when available) before declaring done. + +### High severity — must fix + +- `template-injection` — no `${{ ... }}` from untrusted context inside `run:` or `script:`. +- `dangerous-triggers` — `pull_request_target` / `workflow_run` justified and hardened. +- `unpinned-uses` — every `uses:` has a 40-char SHA + `# vX.Y.Z` comment. +- `excessive-permissions` — workflow- and job-level `permissions:` minimal. +- `secrets-inherit` — no `secrets: inherit`. +- `known-vulnerable-actions` — no pinned versions in GHSA. +- `github-env` — no untrusted writes to `$GITHUB_ENV` / `$GITHUB_PATH`. +- `github-app` — `actions/create-github-app-token` calls scope `repositories:` and `permissions:`. +- `hardcoded-container-credentials` — container `credentials:` use `secrets.*`. + +### Medium severity — fix unless justified + +- `overprovisioned-secrets` — no `${{ toJSON(secrets) }}` or wholesale `${{ secrets }}`. +- `cache-poisoning` — no `actions/cache` (or `setup-*` cache) in release workflows triggered by tags. +- `unredacted-secrets` — no `fromJSON(secrets.*)` paths that get echoed. +- `ref-confusion` — pinned ref isn't a name shared by both a tag and a branch. +- `use-trusted-publishing` — PyPI/npm/etc. publishing uses OIDC trusted publishing. +- `ref-version-mismatch` — the `# vX.Y.Z` comment matches what the SHA actually is. + +### Low severity — fix when reasonable + +- `stale-action-refs` — pinned commit corresponds to a real tag. +- `impostor-commit` — pinned SHA exists in the action repo's history. + +## Verification protocol + +When producing a workflow, report: + +- Every action pinned with `owner/repo@sha # vX.Y.Z`. +- The `permissions:` declared. +- Any checklist item intentionally skipped and why. diff --git a/src/docs/GitHub-Actions/index.md b/src/docs/GitHub-Actions/index.md new file mode 100644 index 0000000..d4a77e7 --- /dev/null +++ b/src/docs/GitHub-Actions/index.md @@ -0,0 +1,15 @@ +# GitHub Actions + +GitHub Actions is a CI/CD and workflow automation platform built into GitHub. Actions are reusable steps that compose into pipelines triggered by repository events — pushes, pull requests, issue comments, schedules, and more. PSModule's actions are composite actions written in PowerShell and published to the [GitHub Marketplace](https://github.com/marketplace?&verification=&query=publisher%3Apsmodule). See the [official GitHub Actions documentation](https://docs.github.com/en/actions). + +Reusable actions are the foundation of a scalable, consistent DevOps practice on GitHub. Encapsulating common tasks — building, testing, releasing, and deploying PowerShell artifacts — into shared, versioned actions eliminates duplication and enforces best practices automatically across every repository in the organization. A single improvement to a shared action benefits all consumers immediately. + +## Getting started + +PSModule actions are built from the [Template-Action](https://github.com/PSModule/Template-Action) repository template, which sets up automated workflows for testing and publishing to the GitHub Marketplace. Each action is a composite action backed by PowerShell logic, following consistent input/output conventions. PR labels control versioning, and the `RepoType: Action` custom property integrates each repository with organization-wide tooling. + +## What we have delivered + + + + diff --git a/src/docs/PowerShell/Standard/Code-Layout.md b/src/docs/PowerShell/Standard/Code-Layout.md new file mode 100644 index 0000000..d66ee1c --- /dev/null +++ b/src/docs/PowerShell/Standard/Code-Layout.md @@ -0,0 +1,189 @@ +# Code layout + +## Brace style (OTBS) + +**Practice:** Opening braces on the same line as the statement. Closing braces on their own line. Always use braces, even for single-statement blocks. + +**Why:** Consistent braces make diffs single-line and prevent the PowerShell parser quirk where a newline before `{` silently breaks constructs. Adding a statement later cannot introduce a logic bug when braces are always present (*Make change easy*). + +**How:** + +```powershell +# Good +function Get-Example { + if ($Name) { + Write-Output "Hello, $Name" + } else { + Write-Output 'Hello, World' + } +} +``` + +```powershell +# Bad +function Get-Example +{ # Brace must be on same line +} + +if ($condition) + Write-Output 'Missing braces' # Always use braces +``` + +## Indentation and whitespace + +**Practice:** 4-space indentation (no tabs). One space around operators and after commas. No trailing whitespace. One blank line between logical blocks. Files end with a single newline. + +**Why:** Reviewers — human and agent — should focus on behaviour, not formatting (*4-eyes principle*). Mechanical consistency removes noise from diffs, keeping structural and behavioural changes visibly separate (*Make change easy*). + +**How:** + +```powershell +# Good +$value = Get-Something -Name 'Test' + +foreach ($item in $Items) { + $processed = Format-Item $item +} +``` + +```powershell +# Bad +$value=Get-Something -Name 'Test' +foreach($item in $Items){ +$processed=Format-Item $item} +``` + +## Line length + +**Practice:** Wrap at 115 characters. Prefer splatting or natural line breaks (after commas, pipes) over backtick continuations. + +**Why:** Backtick continuation is fragile — a trailing space silently breaks it and the bug is invisible in most editors. Splatting achieves the same result deterministically (*Determinism before intelligence*) and diffs cleanly (*Make change easy*). + +**How:** + +```powershell +# Good — splatting +$params = @{ + Path = 'C:\Temp' + Filter = '*.txt' + Recurse = $true + ErrorAction = 'Stop' +} +Get-ChildItem @params + +# Good — natural break after pipe +Get-ChildItem -Path $root | + Where-Object { $_.Length -gt 1MB } +``` + +```powershell +# Bad — backtick continuation +Get-ChildItem -Path 'C:\Temp' ` + -Filter '*.txt' ` + -Recurse +``` + +## Avoid semicolons as line terminators + +**Practice:** Do not use `;` to terminate lines or separate statements within a line. One statement per line. + +**Why:** Semicolons are unnecessary in PowerShell and produce noisy edits when someone later removes them. One statement per line makes diffs atomic and readable (*Make change easy*). + +**How:** + +```powershell +# Good +$name = 'John' +$age = 30 + +$hashtable = @{ + Name = 'John' + Age = 30 +} +``` + +```powershell +# Bad +$name = 'John'; $age = 30 + +$hashtable = @{ Name = 'John'; Age = 30 } +``` + +## Regions + +**Practice:** Use `#region`/`#endregion` to mark logical sections. Label every region. + +**Why:** Regions create named, collapsible blocks in VS Code without premature file-splitting (*DRY — with judgment*). Labels double as in-file documentation (*Documentation lives close to the thing it documents*). + +**How:** + +```powershell +# Good +#region Input validation +if (-not (Test-Path $ConfigPath)) { + throw "Config not found: $ConfigPath" +} +#endregion Input validation + +#region Helper functions +function Get-ConfigData { } +function New-UserAccount { } +#endregion Helper functions +``` + +```powershell +# Bad +#region +function Get-ConfigData { } # Unlabelled — useless for navigation +#endregion +``` + +## Lowercase keywords + +**Practice:** All PowerShell keywords must be lowercase: `function`, `filter`, `param`, `begin`, `process`, `end`, `if`, `else`, `elseif`, `switch`, `foreach`, `for`, `while`, `do`, `return`, `throw`, `try`, `catch`, `finally`. + +**Why:** Consistent casing for language keywords separates them visually from PascalCase user-defined names. Lowercase keywords are the community convention enforced by formatters and linters (*Clean Code*). + +**How:** + +```powershell +# Good +function Get-Item { + param() + begin { } + process { } + end { } +} +``` + +```powershell +# Bad +Function Get-Item { + Param() + Begin { } + Process { } + End { } +} +``` + +## No ternary operators + +**Practice:** Do not use ternary operators (`condition ? a : b`). Use `if`/`else` instead. + +**Why:** Ternary operators are not available in PowerShell 5.1. Using `if`/`else` maintains cross-version compatibility (*Build for all developers*). + +**How:** + +```powershell +# Good +if ($condition) { + $value = 'yes' +} else { + $value = 'no' +} +``` + +```powershell +# Bad +$value = $condition ? 'yes' : 'no' # Breaks on PowerShell 5.1 +``` diff --git a/src/docs/PowerShell/Standard/Control-Flow.md b/src/docs/PowerShell/Standard/Control-Flow.md new file mode 100644 index 0000000..00e8a18 --- /dev/null +++ b/src/docs/PowerShell/Standard/Control-Flow.md @@ -0,0 +1,67 @@ +# Control flow + +## If/else + +**Practice:** Always use braces. `elseif` (one word). One space before `{`. Break complex conditions across lines. + +**Why:** Missing braces cause logic bugs when lines are added later (*Make change easy*). `elseif` is idiomatic PowerShell. + +**How:** + +```powershell +# Good +if ($condition) { + Do-Something +} elseif ($otherCondition) { + Do-SomethingElse +} else { + Do-Default +} +``` + +```powershell +# Bad +if ($condition) +{ # Brace on wrong line + Do-Something +} + +if ($condition) Do-Something # Missing braces +``` + +## Loops + +**Practice:** Prefer `foreach` over `for` when the index is not needed. Always use braces. + +**Why:** `foreach` reads like prose and avoids off-by-one errors (*Clean Code*). Braces protect against accidental modification (*Make change easy*). + +**How:** + +```powershell +# Good +foreach ($item in $collection) { + Process-Item $item +} +``` + +```powershell +# Bad +foreach ($item in $collection) Process-Item $item # Missing braces +``` + +## Switch statements + +**Practice:** Opening brace on same line. Case blocks indented 4 spaces. Always include `default`. + +**Why:** A `default` case makes it explicit that unmatched values are handled intentionally. Unhandled cases are silent failures (*Shift Left*). + +**How:** + +```powershell +# Good +switch ($value) { + 'Option1' { Do-FirstThing } + 'Option2' { Do-SecondThing } + default { Do-DefaultThing } +} +``` diff --git a/src/docs/PowerShell/Standard/Documentation.md b/src/docs/PowerShell/Standard/Documentation.md new file mode 100644 index 0000000..231a433 --- /dev/null +++ b/src/docs/PowerShell/Standard/Documentation.md @@ -0,0 +1,61 @@ +# Documentation and comments + +## Comment-based help + +**Practice:** Comment-based help goes first inside the function body. Include sections in this order: `.SYNOPSIS` (one sentence, imperative mood), `.DESCRIPTION` (one paragraph per behaviour or parameter set), `.EXAMPLE` (at least one per public behaviour), `.INPUTS`, `.OUTPUTS` (matches `[OutputType()]`), `.NOTES`, `.LINK`. Use inline comments above each parameter (not `.PARAMETER`). Comments explain *why*, not *what*. + +**Why:** Documentation lives close to the thing it documents (*Principles*). Help inside the body ensures `Get-Help` always finds it and makes it collapsible in VS Code. Explaining *why* preserves context that code alone cannot express — this is how knowledge becomes shared (*Write it down*, *Context-first development*). + +**How:** + +```powershell +# Good +function New-UserAccount { + <# + .SYNOPSIS + Create a new user account. + + .DESCRIPTION + Creates a new user account with the specified username and email. + + .EXAMPLE + New-UserAccount -UserName 'jdoe' -Email 'jdoe@example.com' + + .INPUTS + None. + + .OUTPUTS + [PSCustomObject] + + .NOTES + Requires admin permissions on the target tenant. + + .LINK + https://psmodule.io/MyModule/Functions/Users/New-UserAccount/ + #> + [CmdletBinding()] + param( + # The username for the new account. + [Parameter(Mandatory)] + [string] $UserName, + + # The email address for the new account. + [Parameter(Mandatory)] + [string] $Email + ) + + # Validate before hitting the database — fail fast + if ($Email -notmatch '^[\w-\.]+@([\w-]+\.)+[\w-]{2,4}$') { + throw "Invalid email format: $Email" + } +} +``` + +```powershell +# Bad +function New-UserAccount { + param($UserName, $Email) + # Check email # States "what", not "why" + if ($Email -notmatch '^[\w-\.]+@([\w-]+\.)+[\w-]{2,4}$') { throw 'Invalid' } +} +``` diff --git a/src/docs/PowerShell/Standard/Function-Structure.md b/src/docs/PowerShell/Standard/Function-Structure.md new file mode 100644 index 0000000..6a7ba82 --- /dev/null +++ b/src/docs/PowerShell/Standard/Function-Structure.md @@ -0,0 +1,167 @@ +# Function structure + +## Always use CmdletBinding + +**Practice:** Every function must have `[CmdletBinding()]`, `[OutputType()]`, and a typed `param()` block with validation attributes. Mandatory parameters first. Use `[switch]` for boolean flags. Add `SupportsShouldProcess` only on commands that mutate state (create, update, remove, copy, install, publish). Never add it to `Get-`, `Test-`, `Resolve-`, `ConvertTo-`, `ConvertFrom-` commands. + +**Why:** `CmdletBinding` gives `-Verbose`, `-WhatIf`, and common parameters for free — features users expect from a well-built product (*Product mindset*). Validation attributes reject bad input at the boundary, not deep in the call stack (*Shift Left*). Typed parameters are self-documenting for agents (*Human–agent coexistence*). + +**How:** + +```powershell +# Good +function Get-UserData { + [OutputType([PSCustomObject])] + [CmdletBinding()] + param( + # The unique identifier of the user. + [Parameter(Mandatory, Position = 0)] + [ValidateNotNullOrEmpty()] + [string] $UserId, + + # Include deleted users in the results. + [Parameter()] + [switch] $IncludeDeleted + ) + + process { } +} +``` + +```powershell +# Bad +function Get-UserData($id, $del) { + # No CmdletBinding, no types, unclear names +} +``` + +## Parameter attribute order + +**Practice:** Place parameter attributes in this order, each on its own line: `[Parameter()]`, validation attributes, `[ArgumentCompleter()]`, `[Alias()]`, typed declaration. + +**Why:** Consistent ordering makes parameter blocks scannable across all functions in a codebase. Reviewers know exactly where to look for each concern (*4-eyes principle*, *Clean Code*). + +**How:** + +```powershell +# Good +param( + [Parameter(Mandatory)] + [ValidateNotNullOrEmpty()] + [ValidateSet('Active', 'Archived')] + [ArgumentCompleter({ Get-StateCompleter })] + [Alias('Status')] + [string] $State +) +``` + +```powershell +# Bad +param( + [Alias('Status')] + [string] + [ValidateSet('Active', 'Archived')] + [Parameter(Mandatory)] + $State # Random order, type on wrong line +) +``` + +## Parameter sets + +**Practice:** Single mode — no named set, no `DefaultParameterSetName`. Multiple modes — every set has an intent-revealing name. Never use `'Default'`, `'ByID'`, or `'__AllParameterSets'` as set names. Set `DefaultParameterSetName` to the most common user intent. + +**Why:** Intent-revealing names document the function's modes directly in the metadata (*Write it down*). Users can discover modes via `Get-Help` without reading the implementation (*Product mindset*). + +**How:** + +```powershell +# Good — multiple parameter sets +function Get-Project { + [CmdletBinding(DefaultParameterSetName = 'List projects')] + param( + [Parameter(ParameterSetName = 'List projects')] + [string] $Filter, + + [Parameter(Mandatory, ParameterSetName = 'Get a project by name')] + [string] $Name + ) +} +``` + +```powershell +# Bad +function Get-Project { + [CmdletBinding(DefaultParameterSetName = 'Default')] + param( + [Parameter(ParameterSetName = 'Default')] + [string] $Filter, + + [Parameter(Mandatory, ParameterSetName = 'ByName')] + [string] $Name + ) +} +``` + +## Pipeline design + +**Practice:** Functions that process collections should accept pipeline input via `ValueFromPipeline` and implement `begin`/`process`/`end` blocks. + +**Why:** Pipeline-aware functions compose with the ecosystem — streaming data without loading entire collections into memory. This follows *Open/Closed*: extend by composing, not modifying. Users expect commands to work naturally in a pipeline (*Product mindset*). + +**How:** + +```powershell +# Good +function Update-Item { + [CmdletBinding()] + param( + [Parameter(Mandatory, ValueFromPipeline)] + [PSCustomObject] $Item + ) + + begin { $count = 0 } + + process { + $Item.LastModified = Get-Date + $count++ + Write-Output $Item + } + + end { Write-Verbose "Processed $count items" } +} + +$items | Update-Item +``` + +## Script structure + +**Practice:** `#Requires` → comment-based help → `param()` → variables → functions → execution in `try`/`catch`. + +**Why:** `#Requires` fails fast on incompatible environments (*Shift Left*). Consistent ordering means agents can reliably parse and generate scripts (*Human–agent coexistence*). A top-level `try`/`catch` prevents partial runs. + +**How:** + +```powershell +# Good +#Requires -Version 7.4 + +[CmdletBinding()] +param( + [string] $ConfigPath = '.\config.json' +) + +$ErrorActionPreference = 'Stop' + +#region Helper functions +function Get-ConfigData { + param([string] $Path) +} +#endregion Helper functions + +try { + $config = Get-ConfigData -Path $ConfigPath +} catch { + Write-Error "Script failed: $_" + exit 1 +} +``` diff --git a/src/docs/PowerShell/Standard/Naming.md b/src/docs/PowerShell/Standard/Naming.md new file mode 100644 index 0000000..6d3fc27 --- /dev/null +++ b/src/docs/PowerShell/Standard/Naming.md @@ -0,0 +1,236 @@ +# Naming + +## Functions and cmdlets + +**Practice:** `Verb-Noun` PascalCase. Approved verbs only (see `Get-Verb`). Singular nouns. Name commands after objects, not lookup mechanisms. Use `filter` only for pure pipeline transforms; use `function` with `begin`/`process`/`end` when setup or cleanup is needed. + +**Why:** Approved verbs make commands discoverable and predictable to any caller — human or agent (*Clean Code*). Discoverable naming is how we *make it easy* for others to do more, faster. + +**How:** + +```powershell +# Good +function Get-UserProfile { } +function Set-ConfigValue { } +function Remove-TempFile { } +Get-ContosoProject -ID 42 # Named after the object +``` + +```powershell +# Bad +function GetUser { } # Missing hyphen +function get-user { } # Wrong case +function Do-Something { } # Unapproved verb +function Get-Users { } # Plural noun +function Get-ContosoProjectByID { } # Named after lookup mechanism +``` + +## Data conversion and I/O verbs + +**Practice:** In data modules, use a fixed verb vocabulary. `ConvertFrom-` turns a format-specific representation into a neutral `[PSCustomObject]`/`[hashtable]`; `ConvertTo-` does the reverse. Use `Import-`/`Export-` for file or store round-trips, `Format-` for a normalized rendering, and `Merge-`, `Compare-`, `Test-`, or `Remove-Entry` to manipulate a structure. Always provide both `ConvertTo-` and `ConvertFrom-` so data can round-trip between the format and the object model. + +**Why:** `ConvertTo`/`ConvertFrom` around the neutral PowerShell object model let any format interoperate with any other through a common pivot — the object — instead of N×N direct converters. A predictable verb set makes a data module's surface obvious to humans and agents. + +**How:** + +```powershell +# Good — the Hashtable module is the reference shape +ConvertFrom-Hashtable # hashtable -> PSCustomObject +ConvertTo-Hashtable # object -> hashtable +Import-Hashtable # file -> hashtable +Export-Hashtable # hashtable -> file +Format-Hashtable # normalized rendering +Merge-Hashtable / Remove-HashtableEntry +``` + +See [Module types](../Modules/Module-Types.md#data-modules). + +## Integration commands and REST methods + +**Practice:** In API/integration modules, name commands after the resource and intent using approved verbs — never after the HTTP method or endpoint path. Map REST methods to verbs: `GET` → `Get-`, `POST` (create) → `New-`/`Add-`, `PUT`/`PATCH` → `Set-`/`Update-`, `DELETE` → `Remove-`, and non-CRUD actions → the approved verb that matches the intent (`Invoke-`, `Start-`, `Stop-`, …). + +**Why:** Users think in resources, not routes. `Get-GitHubRepository` is discoverable; a transport-shaped name like `Invoke-GitHubReposGet` is not. Approved verbs keep the surface consistent across every integration module. + +**How:** + +```powershell +# Good +Get-GitHubRepository -Owner PSModule -Name docs # GET .../repos/PSModule/docs +New-GitHubRepository -Name docs # POST .../repos +Remove-GitHubRepository -Owner PSModule -Name docs # DELETE .../repos/PSModule/docs +``` + +```powershell +# Bad +Invoke-GitHubReposGet # named after the route + HTTP verb +Get-GitHubReposByName # named after the lookup mechanism +``` + +See [Module types](../Modules/Module-Types.md#integration-api-modules). + +## Use full command names + +**Practice:** Always use the full `Verb-Noun` command name. Never use aliases in scripts or shared code. + +**Why:** Not everyone knows the same aliases — `ls`, `dir`, `gci` all mean `Get-ChildItem`. Full names are unambiguous for every reader — including agents and contributors from other platforms (*Build for all developers*, *Clean Code*). + +**How:** + +```powershell +# Good +Get-Process -Name Explorer +Get-ChildItem -Path $root +ForEach-Object { $_.Name } +``` + +```powershell +# Bad +gps -Name Explorer +gci $root +% { $_.Name } +``` + +## Use full parameter names + +**Practice:** Always spell out parameter names explicitly. Do not rely on positional binding or abbreviation. + +**Why:** Explicit parameters make scripts self-documenting and resilient to future parameter-set changes. Readers unfamiliar with the command immediately understand what each argument means (*Clean Code*, *Human–agent coexistence*). + +Enforced by PSScriptAnalyzer rule: `PSAvoidUsingPositionalParameters`. + +**How:** + +```powershell +# Good +Get-Process -Name Explorer +Get-Content -Path $file -TotalCount 10 +Join-Path -Path $PSScriptRoot -ChildPath 'config.json' +``` + +```powershell +# Bad +Get-Process Explorer # Positional — unclear which parameter +Get-Content $file -Tot 10 # Abbreviated — fragile if params change +Join-Path $PSScriptRoot 'config.json' # Positional — flagged by PSAvoidUsingPositionalParameters +``` + +## Parameters and variables + +**Practice:** PascalCase for parameters and public variables. camelCase for local variables. Full words, no abbreviations. Convert external `snake_case` to PascalCase. Avoid repeating the command noun in parameter names. Use the same parameter name for the same concept across a module. Prefix booleans with `is`, `has`, or `should`. Positive switches: `-Force`, `-Recurse`, `-PassThru`. Never shadow automatic variables. Name splats after the call they feed. + +**Why:** Consistent casing communicates scope at a glance (*Clean Code*). Boolean prefixes eliminate guessing. Shadowing automatic variables (`$Error`, `$Args`) causes subtle bugs — catching these at write time is cheaper than in production (*Shift Left*). + +**How:** + +```powershell +# Good +$userName = 'John' +$isValid = $true +$hasPermission = $false +$getProjectSplat = @{ Name = 'Contoso' } + +param( + [string] $ConfigPath, + [switch] $Force +) + +# In Get-ContosoProject, use -ID not -ProjectID +function Get-ContosoProject { + param([int] $ID) +} +``` + +```powershell +# Bad +$usr = 'John' # Too abbreviated +$valid = $true # Should be $isValid +$TOTAL_COUNT = 0 # Wrong case style +$temp = Get-Item . # Meaningless name +$data = @{} # Meaningless name +``` + +## Constants + +**Practice:** PascalCase. Mark with `Set-Variable -Option ReadOnly`. + +**Why:** Read-only marking turns a silent logic error into a visible failure at the earliest moment (*Shift Left*). Constants make intent explicit for anyone reading the code (*Write it down*). + +**How:** + +```powershell +# Good +Set-Variable -Name MaxRetries -Value 3 -Option ReadOnly +Set-Variable -Name DefaultTimeout -Value 30 -Option ReadOnly +``` + +## Classes and enums + +**Practice:** PascalCase for class and enum names. Singular enum names. PascalCase for properties. Boolean properties start with `Is`, `Has`, or `Can`. Sizes in bytes in a property named `Size`. Optional timestamps use `[System.Nullable[datetime]]`. + +**Why:** Consistent casing across types makes the API predictable. Singular enum names align with .NET conventions and read naturally in parameter declarations (*Clean Code*). + +**How:** + +```powershell +# Good +class ContosoProject { + [string] $Name + [bool] $IsActive + [int] $Size + [System.Nullable[datetime]] $ArchivedAt +} + +enum ContosoProjectState { + Active + Archived + Deleted +} +``` + +```powershell +# Bad +class contoso_project { } # Wrong casing +enum ProjectStates { } # Plural +``` + +## File naming + +**Practice:** Filename equals the declared symbol name plus extension. Casing matches the declared symbol. + +**Why:** Predictable file naming enables tooling and humans to locate symbols instantly without searching (*Make change easy*). A 1:1 mapping between file and symbol eliminates ambiguity. + +**How:** + +```powershell +# Good +# Get-ContosoProject.ps1 → contains function Get-ContosoProject +# ContosoProject.ps1 → contains class ContosoProject +# ContosoProject.Types.ps1xml → type extensions for ContosoProject +``` + +```powershell +# Bad +# helpers.ps1 → unclear what's inside +# get-project.ps1 → casing doesn't match function name +``` + +## Use explicit paths + +**Practice:** Use `$PSScriptRoot`-based or absolute paths. Avoid relative paths (`.`, `..`) and `~`. + +**Why:** Relative paths depend on the current location and break when called from .NET methods or other contexts. `~` depends on the current provider and can error unexpectedly. Explicit paths produce deterministic results across all platforms (*Build for all developers*, *Determinism before intelligence*). + +**How:** + +```powershell +# Good +$configPath = Join-Path -Path $PSScriptRoot -ChildPath 'config.json' +Get-Content -Path $configPath +``` + +```powershell +# Bad +Get-Content .\config.json # Relative — depends on $PWD +Get-Content ~\config.json # Provider-dependent +[System.IO.File]::ReadAllText('.\data.txt') # .NET uses a different working directory +``` diff --git a/src/docs/PowerShell/Standard/Output-and-Errors.md b/src/docs/PowerShell/Standard/Output-and-Errors.md new file mode 100644 index 0000000..4b6795e --- /dev/null +++ b/src/docs/PowerShell/Standard/Output-and-Errors.md @@ -0,0 +1,92 @@ +# Output and error handling + +## Output streams + +**Practice:** `Write-Output` for return values. Never `Write-Host`. Use `Write-Verbose`, `Write-Warning`, `Write-Error`, `Write-Information` for their respective purposes. + +**Why:** Each stream has one job (*Single Responsibility*). `Write-Host` bypasses the pipeline, breaks testing, and behaves differently in CI than locally (*Build for all developers*). + +**How:** + +```powershell +# Good +Write-Verbose "Processing $($Data.Count) items" +Write-Output $result +``` + +```powershell +# Bad +Write-Host 'Processing data...' # Bypasses pipeline +Write-Host $result # Untestable +``` + +## Suppressing output + +**Practice:** `$null =` for cmdlets. `[void]` for .NET method calls. Never `| Out-Null`. + +**Why:** `| Out-Null` adds a pipeline stage — measurably slower in loops. `$null =` is resolved at parse time with no runtime cost (*Build for the modern engineer*). + +**How:** + +```powershell +# Good +$null = New-Item -Path $path -ItemType Directory +[void]$list.Add($item) +``` + +```powershell +# Bad +New-Item -Path $path -ItemType Directory | Out-Null +$list.Add($item) | Out-Null +``` + +## Error handling + +**Practice:** Use `try`/`catch`/`finally`. Catch specific exception types. `throw` for unrecoverable errors. `Write-Error` for non-terminating. Use `try`/`catch` only when adding value (translation, enrichment, cleanup). + +**Why:** Silently swallowing exceptions hides bugs — the opposite of *Shift Left*. Specific catch blocks separate recoverable from fatal errors. Meaningful messages serve the next person diagnosing the failure (*Write it down*). + +**How:** + +```powershell +# Good +try { + Get-Content -Path $Path -ErrorAction Stop +} catch [System.IO.IOException] { + Write-Error "IO error: $_" + throw +} catch { + Write-Error "Unexpected error: $_" + throw +} +``` + +```powershell +# Bad +try { + Get-Content -Path $Path +} catch { + # Silently swallowed +} +``` + +## Never pass -Verbose explicitly + +**Practice:** Never pass `-Verbose` to commands inside scripts or module code. The only permitted exception is `-Verbose:$false` to explicitly silence a call. + +**Why:** Passing `-Verbose` overrides the caller's preference, breaking the expected propagation of common parameters. This violates the principle of least surprise and makes verbose output uncontrollable (*Product mindset*). + +**How:** + +```powershell +# Good — verbose propagates from caller preference +Invoke-RestMethod -Uri $uri -Method Get + +# Good — explicitly silencing a noisy call +Import-Module -Name Az.Accounts -Verbose:$false +``` + +```powershell +# Bad +Invoke-RestMethod -Uri $uri -Method Get -Verbose # Overrides caller preference +``` diff --git a/src/docs/PowerShell/Standard/Performance.md b/src/docs/PowerShell/Standard/Performance.md new file mode 100644 index 0000000..1c0ce07 --- /dev/null +++ b/src/docs/PowerShell/Standard/Performance.md @@ -0,0 +1,28 @@ +# Performance + +**Practice:** Use `List[T]` instead of `@()` in loops. Use `-Filter` over `Where-Object`. Use `.Where()` and `.ForEach()` for large collections. + +**Why:** `$results += $item` copies the entire array on every iteration — O(n²). `List[T]` appends in O(1). Provider `-Filter` is evaluated at the source before data crosses the pipeline. Use the construct that scales (*Build for the modern engineer*). + +**How:** + +```powershell +# Good +$results = [System.Collections.Generic.List[PSObject]]::new() +foreach ($item in $collection) { + $results.Add($processedItem) +} + +Get-ChildItem -Path C:\Temp -Filter *.txt +$filtered = $collection.Where({ $_.Value -gt 10 }) +``` + +```powershell +# Bad +$results = @() +foreach ($item in $collection) { + $results += $processedItem # O(n²) +} + +Get-ChildItem -Path C:\Temp | Where-Object { $_.Name -like '*.txt' } +``` diff --git a/src/docs/PowerShell/Standard/Readability.md b/src/docs/PowerShell/Standard/Readability.md new file mode 100644 index 0000000..587ac7c --- /dev/null +++ b/src/docs/PowerShell/Standard/Readability.md @@ -0,0 +1,160 @@ +# Readability + +## String handling + +**Practice:** Single quotes for literals. Double quotes only for expansion. Here-strings for multi-line. `-f` operator for formatting. + +**Why:** Single-quoted strings are literal — no accidental variable expansion. Choosing the correct quote style makes intent explicit at write time (*Shift Left*, *Clean Code*). + +**How:** + +```powershell +# Good +$name = 'John' +$greeting = "Hello, $name" +$path = 'C:\Temp\file.txt' +$message = 'The value is: {0}' -f $value +``` + +```powershell +# Bad +$name = "John" # No expansion needed — use single quotes +$greeting = 'Hello, $name' # Variable will not expand +$message = 'The value is: ' + $value # Use -f operator +``` + +## Comparison operators + +**Practice:** Use PowerShell operators (`-eq`, `-ne`, `-gt`, `-contains`). Place `$null` on the left of null checks. + +**Why:** C-style operators silently produce wrong results. `$null -eq $collection` is safe even when `$collection` is an array. Bugs surface at write time, not in production (*Shift Left*). + +**How:** + +```powershell +# Good +if ($value -eq 10) { } +if ($list -contains $item) { } +if ($null -eq $collection) { } # Safe for arrays +``` + +```powershell +# Bad +if ($value == 10) { } # Not a PowerShell operator +if ($list -eq $item) { } # Use -contains +if ($collection -eq $null) { } # Unsafe on arrays +``` + +## Splatting + +**Practice:** Use splatting when a call has more than two or three parameters. + +**Why:** Long parameter lines are hard to diff and review (*4-eyes principle*). Splatting makes parameters easy to add or remove in isolation (*Make change easy*). + +**How:** + +```powershell +# Good +$params = @{ + Path = 'C:\Temp' + Filter = '*.txt' + Recurse = $true + ErrorAction = 'Stop' +} +Get-ChildItem @params +``` + +```powershell +# Bad +Get-ChildItem -Path 'C:\Temp' -Filter '*.txt' -Recurse $true -ErrorAction 'Stop' +``` + +## Arrays and hashtables + +**Practice:** Use `@()` for arrays, `@{}` for hashtables. One item per line in multi-line collections. Align hashtable values. + +**Why:** Explicit syntax makes intent clear (*Clean Code*). One item per line produces single-line diffs (*Make change easy*). + +**How:** + +```powershell +# Good +$items = @( + 'First' + 'Second' + 'Third' +) + +$config = @{ + Name = 'John' + Age = 30 +} +``` + +```powershell +# Bad +$items = 'First', 'Second', 'Third' +$config = @{Name = 'John'; Age = 30} +``` + +## String emptiness + +**Practice:** Test string emptiness with `-not $Param`. Never use `[string]::IsNullOrEmpty(...)`. + +**Why:** `-not` on a string evaluates to `$true` for both `$null` and empty string, is idiomatic PowerShell, and reads naturally. The .NET method adds noise and is redundant in PowerShell context (*Clean Code*). + +**How:** + +```powershell +# Good +if (-not $Name) { + throw 'Name is required' +} +``` + +```powershell +# Bad +if ([string]::IsNullOrEmpty($Name)) { + throw 'Name is required' +} +``` + +## Wildcard detection + +**Practice:** Use `[System.Management.Automation.WildcardPattern]::ContainsWildcardCharacters($Value)` to detect wildcards. Never use `$Value.Contains('*')`. + +**Why:** The `.Contains('*')` approach misses `?`, `[`, and `` ` `` — all valid PowerShell wildcard characters. The built-in API handles the full wildcard grammar correctly (*Determinism before intelligence*). + +**How:** + +```powershell +# Good +if ([System.Management.Automation.WildcardPattern]::ContainsWildcardCharacters($Path)) { + # Handle wildcard path +} +``` + +```powershell +# Bad +if ($Path.Contains('*')) { + # Misses ?, [, and backtick wildcards +} +``` + +## Cross-platform environment access + +**Practice:** Use `[System.Environment]::ProcessorCount` instead of `$env:NUMBER_OF_PROCESSORS`. Prefer .NET APIs over platform-specific environment variables. + +**Why:** `$env:NUMBER_OF_PROCESSORS` is only available on Windows. The .NET API works reliably on all platforms (*Build for all developers*). + +**How:** + +```powershell +# Good +$cpuCount = [System.Environment]::ProcessorCount +``` + +```powershell +# Bad +$cpuCount = $env:NUMBER_OF_PROCESSORS # Windows-only +``` diff --git a/src/docs/PowerShell/Standard/Security.md b/src/docs/PowerShell/Standard/Security.md new file mode 100644 index 0000000..16db023 --- /dev/null +++ b/src/docs/PowerShell/Standard/Security.md @@ -0,0 +1,29 @@ +# Security + +**Practice:** Never hardcode credentials. Use `SecureString`/`Get-Credential`. Validate all input. Support `-WhatIf`/`-Confirm` for destructive operations. Never `Invoke-Expression` with user-supplied data. + +**Why:** Hardcoded secrets in source control are irreversible — they cannot be fully recalled (*Make change easy* — irreversible decisions deserve more care). `Invoke-Expression` with unsanitized input is a code injection vector. `-WhatIf` lets operators preview destructive changes (*4-eyes principle*). + +**How:** + +```powershell +# Good +function Remove-UserData { + [CmdletBinding(SupportsShouldProcess, ConfirmImpact = 'High')] + param( + [Parameter(Mandatory)] + [ValidateNotNullOrEmpty()] + [string] $UserId + ) + + if ($PSCmdlet.ShouldProcess($UserId, 'Remove user data')) { + # Perform deletion + } +} +``` + +```powershell +# Bad +$password = 'MyPassword123' # Hardcoded secret +Invoke-Expression $userInput # Code injection risk +``` diff --git a/src/docs/PowerShell/Standard/Testing.md b/src/docs/PowerShell/Standard/Testing.md new file mode 100644 index 0000000..07ca4d7 --- /dev/null +++ b/src/docs/PowerShell/Standard/Testing.md @@ -0,0 +1,26 @@ +# Testing + +**Practice:** Pester tests for every function. Files named `*.Tests.ps1`. Structure: `Describe` → `Context` → `It`. Use `BeforeAll`/`AfterAll` for setup. + +**Why:** Tests are the executable specification (*TDD*). They catch regressions before production (*Shift Left*) and give contributors confidence to refactor. Consistent naming lets CI, pre-commit hooks, and editors discover tests automatically (*Testable locally*, *Validatable in PRs*). + +**How:** + +```powershell +# Good +Describe 'Get-UserAccount' { + Context 'When user exists' { + It 'Should return user object' { + $result = Get-UserAccount -UserId '123' + $result | Should -Not -BeNullOrEmpty + $result.UserId | Should -Be '123' + } + } + + Context 'When user does not exist' { + It 'Should throw' { + { Get-UserAccount -UserId '999' } | Should -Throw + } + } +} +``` diff --git a/src/docs/PowerShell/Standard/index.md b/src/docs/PowerShell/Standard/index.md new file mode 100644 index 0000000..a3a69f1 --- /dev/null +++ b/src/docs/PowerShell/Standard/index.md @@ -0,0 +1,20 @@ +# PowerShell standards + +These standards apply to all PowerShell files across PSModule repositories. They implement the +engineering principles defined in [Principles](https://msxorg.github.io/docs/Ways-of-Working/Principles/) — specifically +*Clean Code*, *Make change easy*, *Shift Left*, and *Build for the modern engineer*. + +Each standard states the practice, why it matters to us, and how to apply it. + +| Topic | What it covers | +| ----- | -------------- | +| [Code Layout](Code-Layout.md) | Braces, indentation, line length, semicolons, regions | +| [Naming](Naming.md) | Functions, commands, parameters, variables, constants, paths | +| [Function Structure](Function-Structure.md) | CmdletBinding, pipeline design, script structure | +| [Documentation](Documentation.md) | Comment-based help, inline comments | +| [Readability](Readability.md) | Strings, operators, splatting, collections | +| [Control Flow](Control-Flow.md) | If/else, loops, switch | +| [Output and Errors](Output-and-Errors.md) | Streams, output suppression, error handling | +| [Performance](Performance.md) | Collections, filtering, method syntax | +| [Testing](Testing.md) | Pester structure and conventions | +| [Security](Security.md) | Credentials, input validation, ShouldProcess |