Skip to content

Add default-off Windows GoReleaser before-hook script input - #150

Merged
gontzess merged 2 commits into
mainfrom
steve.gontzes/windows-before-hook-input
Oct 15, 2026
Merged

gontzess merged 2 commits into
mainfrom
steve.gontzes/windows-before-hook-input

Conversation

@gontzess

@gontzess gontzess commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Why

Windows Active Directory artifacts can carry stale PE File/Product versions even when the release tag and --version are correct. The Windows build links the committed .syso without refreshing it; caller pre-jobs cannot supply files to the shared job's fresh tag checkout.

Change

  • Add default-empty windows_pre_build_script; only opted-in Windows configs execute the tagged caller script through GoReleaser before.hooks, in _caller with validated RELEASE_TAG.
  • Share one unchanged safe-path predicate in scripts/validate-safe-relative-path.sh across the three path-input checks, the Windows renderer, and the extra-files loop. Preserve the previous empty guards, trimming, diagnostics, streams and exit behavior.
  • Run guarded path checks immediately after the existing pinned workflow checkout, before normalization: validate-inputs has no files to invoke. No new jobs, dependencies, permissions or unpinned fetches; non-path prerequisites remain early checks.
  • Keep normal clean-tree/tag validation and release arguments. Empty input still renders the previous Windows config byte-for-byte.

Local validation

  • make test workflow-validate passes: existing Go/shell/Python rig, pinned-tag/config contracts, new legacy-equivalence corpus for every path-validator caller, and the hook present/absent/failure tests.
  • The frozen empty-hook rendering remains exactly 1,675 bytes (unchanged SHA-256 c56b99f1025946c8af8adb4da4c8e33fe21a6761e8142f9632f94ea40497b9a7).
  • Equivalence tests compare legacy acceptance, exit codes and stdout/stderr diagnostic bytes, including whitespace/extra-file trimming; default rendering is tested with the actual GNU envsubst renderer.
  • PowerShell fixture tests skip because pwsh is absent. YAML parsing passes; actionlint is not installed locally. No live GoReleaser Pro release or native Windows execution was performed.

Consumer verification story

The AD companion generates and verifies the amd64 COFF resource from the tag, retaining icon/manifest resources and checking numeric and string File/Product fields. Its original real-hook/Windows cross-build check passed with v0.5.13-rc.1+build.2; the companion now also adds an automatic CI executable-verification smoke with v1.2.3-ci.0, preserving source VERSION and the local fallback resource. Verify the resulting executable with:

python scripts/windows_pe_version.py verify --tag "$TAG" --file path/to/baton-active-directory.exe

After release, obtain the Windows ZIP from the published manifest, verify its hash/signature/provenance under normal policy, extract the executable, and repeat that check. Check the MSI-installed payload separately. --version and MSI ProductVersion are not PE-resource proof. The verifier was also exercised against the existing published v0.5.12 ZIP and rejected a mismatched tag; this is not evidence of a deployed new hook.

Related drafts and rollout order

Land and publish the shared input at the normal @v4 reference first, then land AD and baton-admin together. These PRs remain drafts; no release/tag publication is part of this change.

Preserve default release config bytes and run explicitly opted-in caller scripts after GoReleaser tag validation, before Windows compilation.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Superseded — see the current review report for commit 8972a45b81d5

General PR Review: Add default-off Windows GoReleaser before-hook script input

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ce5fc28e14cb.
Review mode: full
View review run

Review Summary

This PR adds an optional windows_pre_build_script input to release.yaml. The input is checked against a strict path pattern in validate-inputs. Windows config rendering now goes through the new scripts/render-goreleaser-windows-config.sh, which checks the pattern again, requires the file to exist in _caller, and adds a before.hooks entry bash ./<path> to the Windows GoReleaser template through ${WINDOWS_BEFORE_HOOKS_BLOCK}. RELEASE_TAG is passed only to the Windows GoReleaser step.

I scanned the full PR diff for security and correctness issues. No repo-local criteria loaded, so I applied only the base criteria.

Injection: the regex ^[A-Za-z0-9._/-]+$ with no .. and no leading / blocks shell metacharacters, YAML quote breaks, $ (envsubst) and {{ (GoReleaser templating). The value reaches the shell only as a quoted environment variable or argument.

Default behavior: when the input is empty, the block is set to "". The renderer always resets it, so a value inherited from the environment can't leak in, and the output matches the old template. The new byte-for-byte fixture test and the added WINDOWS_BEFORE_HOOKS_BLOCK: "" in test-release-config-templates.py cover this.

Hook timing: GoReleaser's git pipe runs before the before pipe, so the docs are right that the clean-tree and tag checks happen before the hook.

set -euo pipefail: adding it to the "Generate configs for Windows" step is safe for the existing commands.

Secrets in the hook: the hook is caller code from the tagged commit, and it runs with the caller-provided RELEASE_TAG/GORELEASER_KEY/RELENG_GITHUB_TOKEN env and the job's OIDC permissions. Anyone who can push a tag can already change the caller's workflows, so this doesn't give anyone new access.

Not reviewed: I didn't check whether GoReleaser on windows-latest resolves the bare bash in the hook to Git Bash, because that can only be confirmed on a real runner. The PR body says no Windows-runner release was performed. A first RC release after rollout is the check for this.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Superseded — see the current review report for commit bd5fa08cff7d

General PR Review: Add default-off Windows GoReleaser before-hook script input

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ce5fc28e14cb.
Review mode: full
View review run

Review Summary

This PR adds an optional windows_pre_build_script input to release.yaml. The input is checked against a strict path pattern in validate-inputs. Windows config rendering now goes through the new scripts/render-goreleaser-windows-config.sh, which checks the pattern again, requires the file to exist in _caller, and adds a before.hooks entry bash ./<path> to the Windows GoReleaser template through ${WINDOWS_BEFORE_HOOKS_BLOCK}. RELEASE_TAG is passed only to the Windows GoReleaser step.

I scanned the full PR diff for security and correctness issues. No repo-local criteria loaded, so I applied only the base criteria.

Injection: the regex ^[A-Za-z0-9._/-]+$ with no .. and no leading / blocks shell metacharacters, YAML quote breaks, $ (envsubst) and {{ (GoReleaser templating). The value reaches the shell only as a quoted environment variable or argument.

Default behavior: when the input is empty, the block is set to "". The renderer always resets it, so a value inherited from the environment can't leak in, and the output matches the old template. The new byte-for-byte fixture test and the added WINDOWS_BEFORE_HOOKS_BLOCK: "" in test-release-config-templates.py cover this.

Hook timing: GoReleaser's git pipe runs before the before pipe, so the docs are right that the clean-tree and tag checks happen before the hook.

set -euo pipefail: adding it to the "Generate configs for Windows" step is safe for the existing commands.

Secrets in the hook: the hook is caller code from the tagged commit, and it runs with the caller-provided RELEASE_TAG/GORELEASER_KEY/RELENG_GITHUB_TOKEN env and the job's OIDC permissions. Anyone who can push a tag can already change the caller's workflows, so this doesn't give anyone new access.

Not reviewed: I didn't check whether GoReleaser on windows-latest resolves the bare bash in the hook to Git Bash, because that can only be confirmed on a real runner. The PR body says no Windows-runner release was performed. A first RC release after rollout is the check for this.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.


Reviewed commit: 8972a45b81d5

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found — see the full review report

Keep the existing predicate, guards, diagnostics and default config bytes while sharing one helper after the already-pinned workflow checkout.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Superseded — see the current review report for commit bd5fa08cff7d

General PR Review: Add default-off Windows GoReleaser before-hook script input

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ce5fc28e14cb.
Review mode: incremental since 8972a45
View review run

Review Summary

The new commit (bd5fa08) puts the safe-relative-path check in one helper, scripts/validate-safe-relative-path.sh. It moves the checks for dockerfile_template, docker_extra_files, msi_wxs_path and windows_pre_build_script out of validate-inputs and into determine-workflows-ref, after the pinned _workflows checkout. The renderer now reuses the same helper, and a new test-safe-relative-path.py compares the helper and every call site against the old Bash rule.

I scanned the full PR diff for security and correctness, including the opt-in before.hooks rendering and how RELEASE_TAG and the script path reach the Windows job.

  • Checks still run before any build: every build job (release-preflight, goreleaser-binaries/-windows/-docker) needs determine-workflows-ref (release.yaml:259,324,654,943). The checks run before Normalize release options, and no permissions change.
  • Same rule as before: the helper's predicate is identical to the old inline one. A wrong argument count exits 2, which every caller treats as invalid.
  • Script path stays contained: the renderer still rejects unsafe paths before the -f check, and the ./ prefix keeps paths starting with - from being read as options.

No trusted repo-local criteria loaded, so only the base criteria were applied. The incremental diff was complete (not partial). No GoReleaser or Windows runner execution was verified (out of scope for a read-only review).

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

General PR Review: Add default-off Windows GoReleaser before-hook script input

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ce5fc28e14cb.
Review mode: incremental since 8972a45
View review run

Review Summary

The new commit (bd5fa08) puts the safe-relative-path check in one helper, scripts/validate-safe-relative-path.sh. It moves the checks for dockerfile_template, docker_extra_files, msi_wxs_path and windows_pre_build_script out of validate-inputs and into determine-workflows-ref, after the pinned _workflows checkout. The renderer now reuses the same helper, and a new test-safe-relative-path.py compares the helper and every call site against the old Bash rule.

I scanned the full PR diff for security and correctness, including the opt-in before.hooks rendering and how RELEASE_TAG and the script path reach the Windows job.

  • Checks still run before any build: every build job (release-preflight, goreleaser-binaries/-windows/-docker) needs determine-workflows-ref (release.yaml:259,324,654,943). The checks run before Normalize release options, and no permissions change.
  • Same rule as before: the helper's predicate is identical to the old inline one. A wrong argument count exits 2, which every caller treats as invalid.
  • Script path stays contained: the renderer still rejects unsafe paths before the -f check, and the ./ prefix keeps paths starting with - from being read as options.

No trusted repo-local criteria loaded, so only the base criteria were applied. The incremental diff was complete (not partial). No GoReleaser or Windows runner execution was verified (out of scope for a read-only review).

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.


Reviewed commit: bd5fa08cff7d

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues found — see the full review report

@gontzess
gontzess marked this pull request as ready for review October 15, 2026 14:48
@gontzess
gontzess merged commit bcb5846 into main Oct 15, 2026
3 checks passed
@gontzess
gontzess deleted the steve.gontzes/windows-before-hook-input branch October 15, 2026 15:36
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.

2 participants