From 3981bb74bbc19ef642707e1e3c96d7c5e6a54969 Mon Sep 17 00:00:00 2001 From: Marius Storhaug Date: Fri, 3 Jul 2026 19:54:40 +0200 Subject: [PATCH 1/9] =?UTF-8?q?=F0=9F=93=96=20[Docs]:=20build=20diagnostic?= =?UTF-8?q?s=20and=20reporting=20into=20the=20GitHub=20Actions=20standard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/docs/Coding-Standards/GitHub-Actions.md | 147 +++++++++++++++++++- src/docs/Coding-Standards/index.md | 2 +- 2 files changed, 147 insertions(+), 2 deletions(-) diff --git a/src/docs/Coding-Standards/GitHub-Actions.md b/src/docs/Coding-Standards/GitHub-Actions.md index 6352999..0a2ddd3 100644 --- a/src/docs/Coding-Standards/GitHub-Actions.md +++ b/src/docs/Coding-Standards/GitHub-Actions.md @@ -1,6 +1,6 @@ --- title: GitHub Actions -description: Workflow authoring — SHA pinning, least-privilege permissions, OIDC, secrets handling, and script extraction. +description: Workflow authoring — SHA pinning, least-privilege permissions, OIDC, secrets handling, script extraction, and diagnostic logging. --- # GitHub Actions @@ -322,3 +322,148 @@ concurrency: than a silent one. - **Give every job and every non-trivial step a `name:`.** Named steps make the Actions UI and logs readable and make failures easy to locate. + +## Build in logging and diagnostics + +An action is a black box until it fails, and the difference between a +five-minute fix and an afternoon of blind re-runs is whether the action already +told you what it did, what it decided, and why. Build that in from the start — +diagnosis is a feature of the action, not something bolted on after the first +incident. + +### Log verbosely, but keep the detail collapsed + +- **Emit enough detail to reconstruct a run without repeating it** — the + resolved inputs, the decision taken at each branch of logic, every external + call and its status, and the counts behind any total. An action that prints + only `done` forces the next reader to re-run it with extra `echo`s just to + learn what happened. +- **Wrap that detail in `::group::` / `::endgroup::` blocks so the log is + collapsed by default.** The reader expands only the group they need, and the + top level stays a short, scannable narrative. Group by phase + (`::group::Resolving inputs`, `::group::Publishing 42 pages`), not per line. +- **Show the headline outside the groups.** The one or two lines that answer + "what happened?" — the counts, the decision, the resulting URL — sit at the + top level, visible without expanding anything. Native `::notice::` and + `::warning::` annotations suit this well: they surface on the run summary page, + not only buried in the log. +- **Gate the deepest tracing behind the runner's debug flag.** When a maintainer + re-runs a job with debug logging enabled, the runner sets `RUNNER_DEBUG=1`; key + the verbose dumps (raw responses, full payloads) off it, and prefer the native + `::debug::` command, which the runner renders only in debug mode. A normal run + then stays readable while a debug re-run reveals everything. + +```yaml +- name: Publish + shell: bash + env: + SPACE: ${{ inputs.space }} + RESPONSE: ${{ steps.api.outputs.body }} + run: | + echo "::group::Resolved configuration" + echo "space = ${SPACE}" + echo "dry-run = ${DRY_RUN:-false}" + echo "::endgroup::" + + # Deep trace only when the job was re-run with debug logging enabled. + if [ "${RUNNER_DEBUG:-}" = "1" ]; then + echo "::debug::raw API response: ${RESPONSE}" + fi + + # Headline stays outside the group — visible without expanding anything. + echo "::notice::Published ${COUNT} page(s) to ${SPACE}" +``` + +### Write a receipt to the step summary + +- **Emit a human-readable receipt to `$GITHUB_STEP_SUMMARY`.** The grouped log + is the *trace*; the step summary is the *result* — a short Markdown table or + list of what the run did (created / updated / skipped counts, the target, + links to what it produced). It renders on the run's summary page, so the + outcome is visible without opening a single log group. +- **Keep the summary a receipt, not a second copy of the log.** Put the headline + numbers there, formatted for a person, with links; leave the blow-by-blow in + the collapsed groups. +- **Surface the same facts as outputs.** Anything worth writing to the summary — + a URL, a count, a pass/fail verdict — is also worth declaring as an `output` + (see [Generalize the action](#generalize-the-action-drive-behaviour-through-inputs-and-outputs)), + so a calling workflow acts on a value instead of scraping the summary. + +```bash +{ + echo "## Documentation publish" + echo "" + echo "| Result | Count |" + echo "| ------- | ----- |" + echo "| Created | ${CREATED} |" + echo "| Updated | ${UPDATED} |" + echo "| Skipped | ${SKIPPED} |" + echo "" + echo "[View the published site](${SITE_URL})" +} >> "$GITHUB_STEP_SUMMARY" +``` + +### Report back on the triggering PR or issue + +The step summary is only seen by someone who opens the run. When the outcome +matters to a person mid-flow — a PR author, an issue reporter — surface the +receipt where they already are. **Which channel is right depends on the event +that triggered the run**, so make reporting conditional on the trigger rather +than assuming one exists. + +- **`pull_request` → a pull-request comment.** Post the receipt to the PR so the + author sees it in the timeline. +- **`issues` / `issue_comment` → an issue comment.** Reply on the issue that + started the run. +- **`push` / `schedule` / `workflow_dispatch` → the step summary, plus a tracking + issue for a finding worth chasing.** There is no PR or issue in context, so the + summary is the receipt; a scheduled job that detects a problem can open or + update an issue. +- **Upsert one comment; never post a fresh one per run.** Write a hidden marker + (an HTML comment such as ``) into the body, find the + existing comment by that marker, and edit it in place — so a PR pushed ten + times carries one current receipt, not ten stale ones. +- **Grant the write scope only on the job that comments.** A PR comment needs + `pull-requests: write`, an issue comment needs `issues: write`; the rest of the + workflow stays read-only. Keep untrusted input out of the comment body (see + [Never expand untrusted input inline](#never-expand-untrusted-input-inline)), + and treat `pull_request_target` with particular care — it runs with a writable + token in the context of untrusted pull-request code. + +```yaml +report: + name: Report + # Only this job can write to the PR; every other job stays read-only. + permissions: + pull-requests: write + if: github.event_name == 'pull_request' + runs-on: ubuntu-24.04 + steps: + - uses: ./.github/actions/publish-receipt # upserts one marked comment +``` + +### Gate merges with a named status check + +- **If a run's result should block merge, it must surface as a named status + check** that branch protection can require. A run that only writes to the log + or a comment cannot hold a pull request; a required check can. +- **Keep the check name stable.** Branch protection matches checks by name, so + renaming the job or workflow silently drops the gate. Pick a clear, permanent + name and treat it as a contract — renaming it later removes the gate without a + trace, so the rename and the branch-protection update must land together. +- **Fail the check for real problems; do not annotate and exit 0.** A gating + action must exit non-zero when it finds a blocking issue — the annotations and + the red summary are for humans, the exit code is what the merge gate reads. + Reserve `::warning::` and `::notice::` for advisory findings that should *not* + hold the pull request. + +```yaml +jobs: + publishable: + name: Publishable # the exact string branch protection requires + runs-on: ubuntu-24.04 + permissions: + contents: read + steps: + - uses: ./.github/actions/validate-publishable # exits 1 on a blocker +``` diff --git a/src/docs/Coding-Standards/index.md b/src/docs/Coding-Standards/index.md index 434fdf7..d7d7180 100644 --- a/src/docs/Coding-Standards/index.md +++ b/src/docs/Coding-Standards/index.md @@ -25,7 +25,7 @@ The baseline pages apply to all code and come first; the per-language standards | [Testing](Testing.md) | The executable specification — test-first, locally runnable, deterministic. | | [Performance](Performance.md) | Scale with the input, measure before optimizing, clarity first. | | [Security](Security.md) | Least privilege, secret hygiene, and the OWASP baseline. | -| [GitHub Actions](GitHub-Actions.md) | Workflow authoring — SHA pinning, least-privilege permissions, OIDC, secrets handling, and script extraction. | +| [GitHub Actions](GitHub-Actions.md) | Workflow authoring — SHA pinning, least-privilege permissions, OIDC, secrets handling, script extraction, and diagnostic logging. | | [Markdown](Markdown.md) | GitHub Flavored Markdown authoring rules enforced by the shared markdownlint configuration. | | [PowerShell](PowerShell/index.md) | Cross-platform PowerShell 7 — the conventions shared by every script, function, and class, with per-construct standards below. | | [Terraform](Terraform.md) | Stack layout, version pinning, state and secrets, and the fmt/validate/tflint toolchain. | From 675a5133dbc0fcdf69e55f4c2079365c13bdb71c Mon Sep 17 00:00:00 2001 From: Marius Storhaug Date: Fri, 3 Jul 2026 20:03:42 +0200 Subject: [PATCH 2/9] =?UTF-8?q?=F0=9F=93=96=20[Docs]:=20escape=20workflow-?= =?UTF-8?q?command=20data;=20make=20the=20logging=20example=20self-consist?= =?UTF-8?q?ent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/docs/Coding-Standards/GitHub-Actions.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/docs/Coding-Standards/GitHub-Actions.md b/src/docs/Coding-Standards/GitHub-Actions.md index 0a2ddd3..6b1506f 100644 --- a/src/docs/Coding-Standards/GitHub-Actions.md +++ b/src/docs/Coding-Standards/GitHub-Actions.md @@ -352,13 +352,22 @@ incident. the verbose dumps (raw responses, full payloads) off it, and prefer the native `::debug::` command, which the runner renders only in debug mode. A normal run then stays readable while a debug re-run reveals everything. +- **Escape dynamic data in workflow commands.** A command such as `::notice::` or + `::debug::` is a single line, so a value carrying `%`, a carriage return, or a + newline must be encoded (`%25`, `%0D`, `%0A`) or it corrupts the command — the + same class of risk as + [expanding untrusted input inline](#never-expand-untrusted-input-inline). Dump a + raw, multi-line payload inside a collapsed `::group::` as plain output, which + needs no escaping, instead of through a command. ```yaml - name: Publish shell: bash env: SPACE: ${{ inputs.space }} + STATUS: ${{ steps.api.outputs.status }} RESPONSE: ${{ steps.api.outputs.body }} + COUNT: ${{ steps.api.outputs.page-count }} run: | echo "::group::Resolved configuration" echo "space = ${SPACE}" @@ -367,7 +376,10 @@ incident. # Deep trace only when the job was re-run with debug logging enabled. if [ "${RUNNER_DEBUG:-}" = "1" ]; then - echo "::debug::raw API response: ${RESPONSE}" + echo "::debug::api status=${STATUS}, ${#RESPONSE} byte response" + echo "::group::Raw API response" + echo "${RESPONSE}" # multi-line payload — plain output, no escaping + echo "::endgroup::" fi # Headline stays outside the group — visible without expanding anything. From 22312f6a5f7db2e5c5e684e374fa53d05132c181 Mon Sep 17 00:00:00 2001 From: Marius Storhaug Date: Fri, 3 Jul 2026 20:06:57 +0200 Subject: [PATCH 3/9] =?UTF-8?q?=F0=9F=93=96=20[Docs]:=20clarify=20how=20de?= =?UTF-8?q?bug=20logging=20is=20enabled=20(ACTIONS=5FSTEP=5FDEBUG=20?= =?UTF-8?q?=E2=86=92=20RUNNER=5FDEBUG)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/docs/Coding-Standards/GitHub-Actions.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/docs/Coding-Standards/GitHub-Actions.md b/src/docs/Coding-Standards/GitHub-Actions.md index 6b1506f..0a4d4f5 100644 --- a/src/docs/Coding-Standards/GitHub-Actions.md +++ b/src/docs/Coding-Standards/GitHub-Actions.md @@ -347,11 +347,13 @@ incident. top level, visible without expanding anything. Native `::notice::` and `::warning::` annotations suit this well: they surface on the run summary page, not only buried in the log. -- **Gate the deepest tracing behind the runner's debug flag.** When a maintainer - re-runs a job with debug logging enabled, the runner sets `RUNNER_DEBUG=1`; key - the verbose dumps (raw responses, full payloads) off it, and prefer the native - `::debug::` command, which the runner renders only in debug mode. A normal run - then stays readable while a debug re-run reveals everything. +- **Gate the deepest tracing behind the runner's debug flag.** Enabling debug + logging — by setting the `ACTIONS_STEP_DEBUG` variable or using the *Re-run + with debug logging* option — surfaces inside a step as `RUNNER_DEBUG=1` (and the + `runner.debug` context); key the verbose dumps (raw responses, full payloads) + off it, and prefer the native `::debug::` command, which the runner renders only + when debug logging is on. A normal run then stays readable while a debug re-run + reveals everything. - **Escape dynamic data in workflow commands.** A command such as `::notice::` or `::debug::` is a single line, so a value carrying `%`, a carriage return, or a newline must be encoded (`%25`, `%0D`, `%0A`) or it corrupts the command — the From 0e6548a51680bbb40401a426bf0cd80a499a0503 Mon Sep 17 00:00:00 2001 From: Marius Storhaug Date: Sat, 4 Jul 2026 10:38:33 +0200 Subject: [PATCH 4/9] =?UTF-8?q?=F0=9F=93=96=20[Docs]:=20log=20everything?= =?UTF-8?q?=20by=20default=20in=20groups;=20annotations=20for=20callouts;?= =?UTF-8?q?=20collapsible=20step=20summaries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/docs/Coding-Standards/GitHub-Actions.md | 152 +++++++++++--------- 1 file changed, 81 insertions(+), 71 deletions(-) diff --git a/src/docs/Coding-Standards/GitHub-Actions.md b/src/docs/Coding-Standards/GitHub-Actions.md index 0a4d4f5..c58532a 100644 --- a/src/docs/Coding-Standards/GitHub-Actions.md +++ b/src/docs/Coding-Standards/GitHub-Actions.md @@ -325,87 +325,93 @@ concurrency: ## Build in logging and diagnostics -An action is a black box until it fails, and the difference between a -five-minute fix and an afternoon of blind re-runs is whether the action already -told you what it did, what it decided, and why. Build that in from the start — -diagnosis is a feature of the action, not something bolted on after the first -incident. - -### Log verbosely, but keep the detail collapsed - -- **Emit enough detail to reconstruct a run without repeating it** — the - resolved inputs, the decision taken at each branch of logic, every external - call and its status, and the counts behind any total. An action that prints - only `done` forces the next reader to re-run it with extra `echo`s just to - learn what happened. -- **Wrap that detail in `::group::` / `::endgroup::` blocks so the log is - collapsed by default.** The reader expands only the group they need, and the - top level stays a short, scannable narrative. Group by phase - (`::group::Resolving inputs`, `::group::Publishing 42 pages`), not per line. -- **Show the headline outside the groups.** The one or two lines that answer - "what happened?" — the counts, the decision, the resulting URL — sit at the - top level, visible without expanding anything. Native `::notice::` and - `::warning::` annotations suit this well: they surface on the run summary page, - not only buried in the log. -- **Gate the deepest tracing behind the runner's debug flag.** Enabling debug - logging — by setting the `ACTIONS_STEP_DEBUG` variable or using the *Re-run - with debug logging* option — surfaces inside a step as `RUNNER_DEBUG=1` (and the - `runner.debug` context); key the verbose dumps (raw responses, full payloads) - off it, and prefer the native `::debug::` command, which the runner renders only - when debug logging is on. A normal run then stays readable while a debug re-run - reveals everything. -- **Escape dynamic data in workflow commands.** A command such as `::notice::` or - `::debug::` is a single line, so a value carrying `%`, a carriage return, or a - newline must be encoded (`%25`, `%0D`, `%0A`) or it corrupts the command — the - same class of risk as - [expanding untrusted input inline](#never-expand-untrusted-input-inline). Dump a - raw, multi-line payload inside a collapsed `::group::` as plain output, which - needs no escaping, instead of through a command. +An action is a black box until it fails. Make every run explain itself — what it +read, what it decided, and what it produced — *by default*, so a failure is +diagnosed from the log you already have rather than from a second run with extra +logging switched on. The craft is keeping all that detail present but out of the +way until someone wants it. + +### Log the whole story by default, collapsed into groups + +- **Log the inputs as resolved, the decisions taken, each external call and its + status, and the outputs produced — on every run, not only under a debug flag.** + These are exactly the details you need when something breaks; emitting them + only when debug logging is enabled means the run that actually failed tells you + nothing, and has to be reproduced before it can be diagnosed. +- **Wrap each phase in a `::group::` / `::endgroup::` block.** Grouping keeps the + detail present but collapsed — there in plain sight, one expand away — so the + top level reads as a short list of phases while the depth sits a click beneath + each. Group by phase (`Resolve inputs`, `Publish 42 pages`), one group per + phase, not one per line. +- **Wrap the group markers in a helper so the script stays declarative.** + Emitting the raw `::group::` / `::endgroup::` lines by hand is noisy; a small + wrapper that takes a title and a block keeps the intent visible. A PowerShell + action gets this from the PSModule `GitHub` module — `LogGroup 'phase' { ... }`. ```yaml - name: Publish shell: bash env: SPACE: ${{ inputs.space }} - STATUS: ${{ steps.api.outputs.status }} - RESPONSE: ${{ steps.api.outputs.body }} - COUNT: ${{ steps.api.outputs.page-count }} run: | - echo "::group::Resolved configuration" + echo "::group::Resolve inputs" echo "space = ${SPACE}" echo "dry-run = ${DRY_RUN:-false}" echo "::endgroup::" - # Deep trace only when the job was re-run with debug logging enabled. - if [ "${RUNNER_DEBUG:-}" = "1" ]; then - echo "::debug::api status=${STATUS}, ${#RESPONSE} byte response" - echo "::group::Raw API response" - echo "${RESPONSE}" # multi-line payload — plain output, no escaping - echo "::endgroup::" - fi + echo "::group::Publish" + # ...every step of the actual work, logged here as it happens... + echo "::endgroup::" +``` - # Headline stays outside the group — visible without expanding anything. - echo "::notice::Published ${COUNT} page(s) to ${SPACE}" +### Call out the result with an annotation + +- **Use `::notice::`, `::warning::`, or `::error::` for the few lines a reader + must not miss** — above all the final result: what was created, or the + pass/fail verdict. Annotations render on the run summary and inline in the + diff, above the collapsed log, so the headline is visible without expanding a + single group. +- **Annotate the outcome, not the progress.** Step-by-step narration belongs in + the grouped log; if every other line is a `::notice::`, the one callout that + matters is lost. Aim to end a run on a single clear annotation. +- **Escape dynamic data in an annotation.** A workflow command is a single line, + so a value carrying `%`, a carriage return, or a newline must be encoded + (`%25`, `%0D`, `%0A`) or it corrupts the command — the same class of risk as + [expanding untrusted input inline](#never-expand-untrusted-input-inline). A + helper that emits the command handles this for you (PowerShell: + `Write-GitHubNotice` / `Write-GitHubError`). + +```bash +# On success — the one line that answers "what happened?" +echo "::notice title=Published::created ${CREATED}, updated ${UPDATED}" + +# On a blocking failure — annotate, then exit non-zero (see the status check below). +echo "::error title=Publish failed::${FAILED} page(s) rejected" ``` -### Write a receipt to the step summary - -- **Emit a human-readable receipt to `$GITHUB_STEP_SUMMARY`.** The grouped log - is the *trace*; the step summary is the *result* — a short Markdown table or - list of what the run did (created / updated / skipped counts, the target, - links to what it produced). It renders on the run's summary page, so the - outcome is visible without opening a single log group. -- **Keep the summary a receipt, not a second copy of the log.** Put the headline - numbers there, formatted for a person, with links; leave the blow-by-blow in - the collapsed groups. -- **Surface the same facts as outputs.** Anything worth writing to the summary — - a URL, a count, a pass/fail verdict — is also worth declaring as an `output` - (see [Generalize the action](#generalize-the-action-drive-behaviour-through-inputs-and-outputs)), +### Report the bigger picture in a step summary + +- **When the result is more than a line — a table, per-item counts, a report — + write it to `$GITHUB_STEP_SUMMARY` as GitHub-flavored Markdown.** The step + summary renders on the run's summary page, so the outcome is legible without + opening the log at all. +- **Keep the summary uncluttered by default.** Show the headline — the table or + the verdict — in the open, and tuck long or secondary detail inside + `
...` blocks that stay closed until the reader + opens them. A summary that spills everything inline is as hard to scan as an + ungrouped log. +- **Compose the Markdown with a helper rather than building strings** where you + can. A PowerShell action assembles the summary with the PSModule `Markdown` + module — `Heading`, `Table`, `Details { ... }` — and writes it with the + `GitHub` module's `Set-GitHubStepSummary`. +- **Surface the same facts as outputs.** A URL, a count, or a verdict worth + putting in the summary is also worth an `output` (see + [Generalize the action](#generalize-the-action-drive-behaviour-through-inputs-and-outputs)), so a calling workflow acts on a value instead of scraping the summary. ```bash { - echo "## Documentation publish" + echo "## ✅ Documentation publish" echo "" echo "| Result | Count |" echo "| ------- | ----- |" @@ -413,7 +419,11 @@ incident. echo "| Updated | ${UPDATED} |" echo "| Skipped | ${SKIPPED} |" echo "" - echo "[View the published site](${SITE_URL})" + echo "
Pages created (${CREATED})" + echo "" + echo "${CREATED_LIST}" # one entry per line — kept closed until opened + echo "" + echo "
" } >> "$GITHUB_STEP_SUMMARY" ``` @@ -421,22 +431,22 @@ incident. The step summary is only seen by someone who opens the run. When the outcome matters to a person mid-flow — a PR author, an issue reporter — surface the -receipt where they already are. **Which channel is right depends on the event +result where they already are. **Which channel is right depends on the event that triggered the run**, so make reporting conditional on the trigger rather than assuming one exists. -- **`pull_request` → a pull-request comment.** Post the receipt to the PR so the +- **`pull_request` → a pull-request comment.** Post the summary to the PR so the author sees it in the timeline. - **`issues` / `issue_comment` → an issue comment.** Reply on the issue that started the run. - **`push` / `schedule` / `workflow_dispatch` → the step summary, plus a tracking issue for a finding worth chasing.** There is no PR or issue in context, so the - summary is the receipt; a scheduled job that detects a problem can open or - update an issue. + step summary stands on its own; a scheduled job that detects a problem can open + or update an issue. - **Upsert one comment; never post a fresh one per run.** Write a hidden marker - (an HTML comment such as ``) into the body, find the + (an HTML comment such as ``) into the body, find the existing comment by that marker, and edit it in place — so a PR pushed ten - times carries one current receipt, not ten stale ones. + times carries one current comment, not ten stale ones. - **Grant the write scope only on the job that comments.** A PR comment needs `pull-requests: write`, an issue comment needs `issues: write`; the rest of the workflow stays read-only. Keep untrusted input out of the comment body (see @@ -453,7 +463,7 @@ report: if: github.event_name == 'pull_request' runs-on: ubuntu-24.04 steps: - - uses: ./.github/actions/publish-receipt # upserts one marked comment + - uses: ./.github/actions/publish-summary # upserts one marked comment ``` ### Gate merges with a named status check From e7b4bc0d822b940d1c86c5cb7f7a570a8d71c07d Mon Sep 17 00:00:00 2001 From: Marius Storhaug Date: Sat, 4 Jul 2026 11:00:49 +0200 Subject: [PATCH 5/9] =?UTF-8?q?=F0=9F=93=96=20[Docs]:=20address=20review?= =?UTF-8?q?=20=E2=80=94=20DRY=5FRUN=20in=20env,=20checkout=20before=20loca?= =?UTF-8?q?l=20actions,=20escaping=20note,=20secret-safe=20logging,=20jobs?= =?UTF-8?q?=20wrapper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/docs/Coding-Standards/GitHub-Actions.md | 45 ++++++++++++--------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/docs/Coding-Standards/GitHub-Actions.md b/src/docs/Coding-Standards/GitHub-Actions.md index c58532a..149017e 100644 --- a/src/docs/Coding-Standards/GitHub-Actions.md +++ b/src/docs/Coding-Standards/GitHub-Actions.md @@ -333,11 +333,11 @@ way until someone wants it. ### Log the whole story by default, collapsed into groups -- **Log the inputs as resolved, the decisions taken, each external call and its - status, and the outputs produced — on every run, not only under a debug flag.** - These are exactly the details you need when something breaks; emitting them - only when debug logging is enabled means the run that actually failed tells you - nothing, and has to be reproduced before it can be diagnosed. +- **Log the resolved inputs, the decisions taken, each external call and its + status, and the outputs produced — on every run.** These are the details a + failure is diagnosed from, so the first run to fail carries enough to diagnose + it. Log a secret's presence, never its value; secret inputs stay masked (see + [Distinguish `vars` from `secrets`](#distinguish-vars-from-secrets)). - **Wrap each phase in a `::group::` / `::endgroup::` block.** Grouping keeps the detail present but collapsed — there in plain sight, one expand away — so the top level reads as a short list of phases while the depth sits a click beneath @@ -353,10 +353,11 @@ way until someone wants it. shell: bash env: SPACE: ${{ inputs.space }} + DRY_RUN: ${{ inputs.dry-run }} run: | echo "::group::Resolve inputs" echo "space = ${SPACE}" - echo "dry-run = ${DRY_RUN:-false}" + echo "dry-run = ${DRY_RUN}" echo "::endgroup::" echo "::group::Publish" @@ -368,8 +369,8 @@ way until someone wants it. - **Use `::notice::`, `::warning::`, or `::error::` for the few lines a reader must not miss** — above all the final result: what was created, or the - pass/fail verdict. Annotations render on the run summary and inline in the - diff, above the collapsed log, so the headline is visible without expanding a + pass/fail verdict. Annotations render on the run summary and in the Checks + view, above the collapsed log, so the headline is visible without expanding a single group. - **Annotate the outcome, not the progress.** Step-by-step narration belongs in the grouped log; if every other line is a `::notice::`, the one callout that @@ -382,10 +383,11 @@ way until someone wants it. `Write-GitHubNotice` / `Write-GitHubError`). ```bash -# On success — the one line that answers "what happened?" +# Integer counts are safe to interpolate. Escape free-form text (names, messages) +# with %25 / %0D / %0A first, or emit it through a helper such as Write-GitHubNotice. echo "::notice title=Published::created ${CREATED}, updated ${UPDATED}" -# On a blocking failure — annotate, then exit non-zero (see the status check below). +# A blocking failure: annotate, then exit non-zero (see the status check below). echo "::error title=Publish failed::${FAILED} page(s) rejected" ``` @@ -455,15 +457,18 @@ than assuming one exists. token in the context of untrusted pull-request code. ```yaml -report: - name: Report - # Only this job can write to the PR; every other job stays read-only. - permissions: - pull-requests: write - if: github.event_name == 'pull_request' - runs-on: ubuntu-24.04 - steps: - - uses: ./.github/actions/publish-summary # upserts one marked comment +jobs: + report: + name: Report + if: github.event_name == 'pull_request' + runs-on: ubuntu-24.04 + # Only this job can write to the PR; every other job stays read-only. + permissions: + pull-requests: write + steps: + # A local action runs from the checked-out repo, so check out first. + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + - uses: ./.github/actions/publish-summary # upserts one marked comment ``` ### Gate merges with a named status check @@ -489,5 +494,7 @@ jobs: permissions: contents: read steps: + # A local action runs from the checked-out repo, so check out first. + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 - uses: ./.github/actions/validate-publishable # exits 1 on a blocker ``` From 64fbd0f1d8c52598b7e0805b67c2294816626aa6 Mon Sep 17 00:00:00 2001 From: Marius Storhaug Date: Sat, 4 Jul 2026 11:15:13 +0200 Subject: [PATCH 6/9] =?UTF-8?q?=F0=9F=93=96=20[Docs]:=20use=20"pull=20requ?= =?UTF-8?q?est"=20consistently=20in=20prose?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/docs/Coding-Standards/GitHub-Actions.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/docs/Coding-Standards/GitHub-Actions.md b/src/docs/Coding-Standards/GitHub-Actions.md index 149017e..4363df3 100644 --- a/src/docs/Coding-Standards/GitHub-Actions.md +++ b/src/docs/Coding-Standards/GitHub-Actions.md @@ -437,7 +437,7 @@ result where they already are. **Which channel is right depends on the event that triggered the run**, so make reporting conditional on the trigger rather than assuming one exists. -- **`pull_request` → a pull-request comment.** Post the summary to the PR so the +- **`pull_request` → a pull request comment.** Post the summary to the PR so the author sees it in the timeline. - **`issues` / `issue_comment` → an issue comment.** Reply on the issue that started the run. @@ -454,7 +454,7 @@ than assuming one exists. workflow stays read-only. Keep untrusted input out of the comment body (see [Never expand untrusted input inline](#never-expand-untrusted-input-inline)), and treat `pull_request_target` with particular care — it runs with a writable - token in the context of untrusted pull-request code. + token in the context of untrusted pull request code. ```yaml jobs: From 51428160767c9e6f33f2d0fc1a108ad0fa2bf53d Mon Sep 17 00:00:00 2001 From: Marius Storhaug Date: Sun, 5 Jul 2026 12:47:49 +0200 Subject: [PATCH 7/9] =?UTF-8?q?=F0=9F=93=96=20[Docs]:=20default=20to=20Pow?= =?UTF-8?q?erShell=20as=20the=20glue=20language=20in=20the=20GitHub=20Acti?= =?UTF-8?q?ons=20standard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/docs/Coding-Standards/GitHub-Actions.md | 48 ++++++++++++++++++++- src/docs/Coding-Standards/index.md | 2 +- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/docs/Coding-Standards/GitHub-Actions.md b/src/docs/Coding-Standards/GitHub-Actions.md index 4363df3..4754e1f 100644 --- a/src/docs/Coding-Standards/GitHub-Actions.md +++ b/src/docs/Coding-Standards/GitHub-Actions.md @@ -1,6 +1,6 @@ --- title: GitHub Actions -description: Workflow authoring — SHA pinning, least-privilege permissions, OIDC, secrets handling, script extraction, and diagnostic logging. +description: Workflow authoring — SHA pinning, least-privilege permissions, OIDC, secrets handling, a PowerShell-first scripting default, script extraction, and diagnostic logging. --- # GitHub Actions @@ -147,6 +147,52 @@ as script. - run: echo "${{ github.event.pull_request.title }}" ``` +## Default to PowerShell as the glue language + +Between the declarative steps, a workflow always has some glue to run — reading a +value, calling an API, shaping a result. That glue is written in **one language +by default**, so every action reads the same way and reaches for the same helpers +instead of each one inventing its own idiom. **PowerShell is that default.** It +runs cross-platform on every runner, it is held to the +[PowerShell](PowerShell/index.md) standard like any other code, and it carries the +ecosystem's Actions tooling — above all the PSModule `GitHub` module, whose +helpers speak the runner's own workflow-command protocol. + +Reach for the options in this order, and drop to the next only when the one above +genuinely cannot serve: + +- **First choice — PowerShell with the PSModule `GitHub` module.** Talk to the + runner through the module's commands instead of hand-writing raw workflow + strings: the `Write-GitHub*` family (`Write-GitHubNotice`, `Write-GitHubWarning`, + `Write-GitHubError`) for annotations, `LogGroup` for grouped logging, and + `Set-GitHubStepSummary` for the step summary. The helper escapes dynamic data + for you, keeps the script declarative, and gives every action one vocabulary for + the diagnostics the [logging section](#build-in-logging-and-diagnostics) calls + for. +- **Fallback — plain PowerShell.** When the `GitHub` module is not available or + not warranted — a script that does no runner communication, or one that must run + before the module is installed — stay in PowerShell and write to the log + directly. +- **Last resort — Bash, only when PowerShell cannot be supported.** A context with + no `pwsh` — a minimal container, or a step that must run before PowerShell is on + the image — falls back to Bash. Keep it small and hold it to the same rules, + above all [never expand untrusted input inline](#never-expand-untrusted-input-inline). + +```yaml +# Preferred — PowerShell glue; the GitHub module speaks to the runner. +- name: Publish + shell: pwsh + env: + SPACE: ${{ inputs.space }} + DRY_RUN: ${{ inputs.dry-run }} + run: | + LogGroup 'Resolve inputs' { + Write-Host "space = $env:SPACE" + Write-Host "dry-run = $env:DRY_RUN" + } + Write-GitHubNotice -Message "publishing to $env:SPACE" -Title 'Publish' +``` + ## Extract non-trivial `run:` scripts into an action A short `run:` step — a handful of commands wiring tools together — belongs diff --git a/src/docs/Coding-Standards/index.md b/src/docs/Coding-Standards/index.md index d7d7180..c697181 100644 --- a/src/docs/Coding-Standards/index.md +++ b/src/docs/Coding-Standards/index.md @@ -25,7 +25,7 @@ The baseline pages apply to all code and come first; the per-language standards | [Testing](Testing.md) | The executable specification — test-first, locally runnable, deterministic. | | [Performance](Performance.md) | Scale with the input, measure before optimizing, clarity first. | | [Security](Security.md) | Least privilege, secret hygiene, and the OWASP baseline. | -| [GitHub Actions](GitHub-Actions.md) | Workflow authoring — SHA pinning, least-privilege permissions, OIDC, secrets handling, script extraction, and diagnostic logging. | +| [GitHub Actions](GitHub-Actions.md) | Workflow authoring — SHA pinning, least-privilege permissions, OIDC, secrets handling, a PowerShell-first scripting default, script extraction, and diagnostic logging. | | [Markdown](Markdown.md) | GitHub Flavored Markdown authoring rules enforced by the shared markdownlint configuration. | | [PowerShell](PowerShell/index.md) | Cross-platform PowerShell 7 — the conventions shared by every script, function, and class, with per-construct standards below. | | [Terraform](Terraform.md) | Stack layout, version pinning, state and secrets, and the fmt/validate/tflint toolchain. | From 6ecbc73d264539ef6364ce95c9c1a9d517388315 Mon Sep 17 00:00:00 2001 From: Marius Storhaug Date: Sun, 5 Jul 2026 12:56:42 +0200 Subject: [PATCH 8/9] =?UTF-8?q?=F0=9F=93=96=20[Docs]:=20escape=20workflow-?= =?UTF-8?q?command=20properties=20(:/,)=20and=20grant=20contents:=20read?= =?UTF-8?q?=20to=20the=20report=20job?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/docs/Coding-Standards/GitHub-Actions.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/docs/Coding-Standards/GitHub-Actions.md b/src/docs/Coding-Standards/GitHub-Actions.md index 4754e1f..90a9234 100644 --- a/src/docs/Coding-Standards/GitHub-Actions.md +++ b/src/docs/Coding-Standards/GitHub-Actions.md @@ -425,6 +425,8 @@ way until someone wants it. so a value carrying `%`, a carriage return, or a newline must be encoded (`%25`, `%0D`, `%0A`) or it corrupts the command — the same class of risk as [expanding untrusted input inline](#never-expand-untrusted-input-inline). A + value placed in a command *property* (such as `title=`) needs `:` and `,` + encoded too (`%3A`, `%2C`), since those characters delimit the property list. A helper that emits the command handles this for you (PowerShell: `Write-GitHubNotice` / `Write-GitHubError`). @@ -510,7 +512,8 @@ jobs: runs-on: ubuntu-24.04 # Only this job can write to the PR; every other job stays read-only. permissions: - pull-requests: write + contents: read # checkout + pull-requests: write # comment on the PR steps: # A local action runs from the checked-out repo, so check out first. - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 From beb545e41d6d1323fd70747319e9bfcfa8d50146 Mon Sep 17 00:00:00 2001 From: Marius Storhaug Date: Sun, 5 Jul 2026 13:04:03 +0200 Subject: [PATCH 9/9] =?UTF-8?q?=F0=9F=93=96=20[Docs]:=20warn=20against=20w?= =?UTF-8?q?orkflow-command=20injection=20when=20logging=20untrusted=20valu?= =?UTF-8?q?es?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/docs/Coding-Standards/GitHub-Actions.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/docs/Coding-Standards/GitHub-Actions.md b/src/docs/Coding-Standards/GitHub-Actions.md index 90a9234..f3abb45 100644 --- a/src/docs/Coding-Standards/GitHub-Actions.md +++ b/src/docs/Coding-Standards/GitHub-Actions.md @@ -384,6 +384,12 @@ way until someone wants it. failure is diagnosed from, so the first run to fail carries enough to diagnose it. Log a secret's presence, never its value; secret inputs stay masked (see [Distinguish `vars` from `secrets`](#distinguish-vars-from-secrets)). +- **Force an untrusted value onto a single line before logging it.** The runner + parses every line of stdout, so a value you do not control that carries a + newline followed by `::...` can smuggle in a workflow command. Strip or encode + `\r` / `\n` (or emit the value through a helper) so a logged input cannot break + out into a command — the same untrusted-input rule as + [Never expand untrusted input inline](#never-expand-untrusted-input-inline). - **Wrap each phase in a `::group::` / `::endgroup::` block.** Grouping keeps the detail present but collapsed — there in plain sight, one expand away — so the top level reads as a short list of phases while the depth sits a click beneath