From 8972a45b81d53cc52a8b52f9e7f0bc5fc32c1841 Mon Sep 17 00:00:00 2001 From: Steve Gontzes Date: Wed, 14 Oct 2026 16:29:02 +0000 Subject: [PATCH 1/2] feat: add opt-in Windows release pre-build hook 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] --- .github/workflows/release.yaml | 20 ++- Makefile | 1 + README.md | 24 +++ docs/release-workflow.md | 25 +++ scripts/render-goreleaser-windows-config.sh | 27 ++++ scripts/test-release-config-templates.py | 1 + scripts/test-windows-pre-build-script.py | 153 ++++++++++++++++++ .../testdata/goreleaser-windows-default.yaml | 75 +++++++++ .../.goreleaser-windows-template.yaml.tmpl | 2 +- 9 files changed, 326 insertions(+), 2 deletions(-) create mode 100755 scripts/render-goreleaser-windows-config.sh create mode 100755 scripts/test-windows-pre-build-script.py create mode 100644 scripts/testdata/goreleaser-windows-default.yaml diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 9932ab0..b6d657e 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -77,6 +77,11 @@ on: type: string default: "" description: "Path to a custom WXS file in the caller repo for MSI generation (relative to repo root). If not provided, uses default template." + windows_pre_build_script: + required: false + type: string + default: "" + description: "Optional safe relative Bash script in the caller repo, run by GoReleaser before Windows compilation." brew: required: false type: boolean @@ -185,6 +190,16 @@ jobs: exit 1 fi + - name: Validate windows_pre_build_script has safe path + if: inputs.windows_pre_build_script != '' + env: + WINDOWS_PRE_BUILD_SCRIPT: ${{ inputs.windows_pre_build_script }} + run: | + if [[ "$WINDOWS_PRE_BUILD_SCRIPT" == /* ]] || [[ "$WINDOWS_PRE_BUILD_SCRIPT" == *".."* ]] || [[ ! "$WINDOWS_PRE_BUILD_SCRIPT" =~ ^[A-Za-z0-9._/-]+$ ]]; then + echo "::error::windows_pre_build_script must be a relative path with safe characters and without '..' traversal. Got: $WINDOWS_PRE_BUILD_SCRIPT" + exit 1 + fi + - name: Validate GORELEASER_PRO_KEY when msi enabled if: inputs.msi == true env: @@ -747,11 +762,13 @@ jobs: WXS_PATH: ${{ steps.wxs.outputs.wxs_path }} WORKFLOWS_REF: ${{ needs.determine-workflows-ref.outputs.ref }} RELEASE_TAG: ${{ inputs.tag }} + WINDOWS_PRE_BUILD_SCRIPT: ${{ inputs.windows_pre_build_script }} run: | + set -euo pipefail export BUILD_STARTED_ON=$(date -u +"%Y-%m-%dT%H:%M:%SZ") # Generate GoReleaser config - envsubst < templates/.goreleaser-windows-template.yaml.tmpl | tee "_generated/.goreleaser.windows.yaml" + bash ./scripts/render-goreleaser-windows-config.sh "$WINDOWS_PRE_BUILD_SCRIPT" ../_caller | tee "_generated/.goreleaser.windows.yaml" # Generate provenance predicate envsubst < templates/.slsa-provenance-predicate-template.json.tmpl | tee "_generated/predicate.json" @@ -766,6 +783,7 @@ jobs: env: GITHUB_TOKEN: ${{ secrets.RELENG_GITHUB_TOKEN }} GORELEASER_CURRENT_TAG: ${{ inputs.tag }} + RELEASE_TAG: ${{ inputs.tag }} GORELEASER_KEY: ${{ secrets.GORELEASER_PRO_KEY }} - name: Flatten MSI directory structure diff --git a/Makefile b/Makefile index 621f927..777242c 100644 --- a/Makefile +++ b/Makefile @@ -21,6 +21,7 @@ test-scripts: bash scripts/test-derive-iam-role-name.sh bash scripts/test-normalize-release-options.sh python3 scripts/test-release-config-templates.py + python3 scripts/test-windows-pre-build-script.py bash scripts/test-release-workflow-tag-pin.sh bash scripts/test-s3-release-uploads.sh bash scripts/test-validate-release-artifacts.sh diff --git a/README.md b/README.md index 8ce9401..57488d0 100644 --- a/README.md +++ b/README.md @@ -156,6 +156,7 @@ The release workflow accepts the following input parameters: | `docker_extra_files` | No | `""` | Comma-separated list of extra files/dirs to include in Docker build context | | `msi` | No | `true` | Whether to build MSI Windows installers | | `msi_wxs_path` | No | `""` | Path to custom WXS template for MSI installer (uses default if not set) | +| `windows_pre_build_script` | No | `""` | Optional safe repo-relative Bash script, run in the caller checkout by GoReleaser before Windows compilation with `RELEASE_TAG` set to the requested tag. | | `brew` | No | `true` | Whether to publish a Homebrew formula to the selected public `conductorone` tap | 2. Ensure your repository has the following secrets configured: @@ -250,6 +251,29 @@ To disable MSI builds entirely (e.g., for connectors that don't need Windows ins When `msi: false`, the `GORELEASER_PRO_KEY` secret is not required. +### Windows Pre-build Script + +Connectors that generate Windows-specific resources from the release tag can opt in: + +```yaml + with: + tag: ${{ github.ref_name }} + windows_pre_build_script: scripts/release-windows-resources.sh +``` + +The script must exist as a file in the tagged caller repository. Paths must use +only letters, digits, `.`, `_`, `/`, and `-`, and must not be absolute or contain +`..` or whitespace. GoReleaser runs `bash ./` in `_caller` as a +`before.hooks` command, after its normal clean-tree/tag validation and before +Windows compilation. The hook receives the validated workflow tag as +`RELEASE_TAG`; a failing hook fails the Windows release. + +This is Windows-only and opt-in: an empty input leaves the rendered Windows +config unchanged and does not run any generation. macOS, Linux, and Docker +builds do not run this hook. Keep connector-specific generation in the caller's +script; do not skip release validation, use snapshots, retag, or commit generated +resources during release. + ## Verify Workflow Runs linting, tests, and optional regression verification. See [detailed documentation](docs/verify-workflow.md) for jobs, regression testing, and all options. diff --git a/docs/release-workflow.md b/docs/release-workflow.md index a0b432a..74373e0 100644 --- a/docs/release-workflow.md +++ b/docs/release-workflow.md @@ -28,6 +28,8 @@ Validates workflow inputs before proceeding: - Ensures `dockerfile_template` is only used when `lambda: false` - Ensures `docker_extra_files` is only used when `dockerfile_template` is set - Ensures `msi_wxs_path` has no path traversal (`..` or absolute paths) +- Ensures `windows_pre_build_script` is a safe relative path without absolute + paths, `..` traversal, whitespace, or shell metacharacters - Ensures `GORELEASER_PRO_KEY` is provided when `msi: true` ### determine-workflows-ref @@ -69,6 +71,7 @@ Builds Windows zip and MSI installer: - MSI built using WiX Toolset with GoReleaser Pro - Deterministic UpgradeCode via UUID v5 from repository name - Supports custom WXS templates via `msi_wxs_path` input +- Supports an opt-in caller Bash script via `windows_pre_build_script` - Generates SBOMs and SLSA v1 provenance attestations - Uploads all artifacts to S3 with no-overwrite writes @@ -76,6 +79,18 @@ Builds Windows zip and MSI installer: **Custom WXS:** For connectors requiring custom MSI behavior (Windows Service, registry keys, etc.), provide a custom WXS template via `msi_wxs_path`. If not provided, uses the default CLI installer template. +**Windows pre-build script:** `windows_pre_build_script` defaults to empty, +preserving the original rendered GoReleaser config without a hook. When set, +Windows config generation verifies that the safe repo-relative path exists as a +file in `_caller`. The config adds a `before.hooks` command, `bash ./`, +which GoReleaser runs in that same tagged caller checkout after its normal +clean-tree/tag validation and before compilation. Only the Windows GoReleaser +action forwards `RELEASE_TAG` from the validated `tag` input to the hook. Hook +failures fail the release; generated resources remain local to the Windows +build workspace. The workflow does not run this hook on macOS, Linux, Docker, +or callers with the default empty input, and does not skip validation, build +snapshots, retag, or commit generated resources. + ### goreleaser-docker (Linux) Builds and publishes container images: @@ -293,6 +308,16 @@ Use test connectors for validation: ./scripts/validate-release-artifacts.sh ConductorOne/baton-github-test v0.1.xxx ``` +### Local Windows Hook Checks + +`make test-scripts` includes `python3 scripts/test-windows-pre-build-script.py`. +It uses the real workflow Bash steps and `envsubst`, with `yq` for YAML queries: +empty input must match the frozen pre-hook config fixture byte-for-byte; opted-in +scripts must render the expected command; unsafe and missing paths must fail. +The fixture hook also checks the caller working directory, release-tag +forwarding, and command failure propagation without compiling or publishing. +These are local fixture checks, not a deployed Windows release proof. + ### Validation Script The `scripts/validate-release-artifacts.sh` script validates: diff --git a/scripts/render-goreleaser-windows-config.sh b/scripts/render-goreleaser-windows-config.sh new file mode 100755 index 0000000..70f5c90 --- /dev/null +++ b/scripts/render-goreleaser-windows-config.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [ "$#" -ne 2 ]; then + echo "usage: $0 " >&2 + exit 2 +fi + +windows_pre_build_script="$1" +caller_root="$2" +WINDOWS_BEFORE_HOOKS_BLOCK="" + +if [ -n "$windows_pre_build_script" ]; then + if [[ "$windows_pre_build_script" == /* ]] || [[ "$windows_pre_build_script" == *".."* ]] || [[ ! "$windows_pre_build_script" =~ ^[A-Za-z0-9._/-]+$ ]]; then + echo "::error::windows_pre_build_script must be a relative path with safe characters and without '..' traversal. Got: $windows_pre_build_script" >&2 + exit 1 + fi + if [ ! -f "$caller_root/$windows_pre_build_script" ]; then + echo "::error::Windows pre-build script not found: $windows_pre_build_script" >&2 + exit 1 + fi + printf -v WINDOWS_BEFORE_HOOKS_BLOCK 'before:\n hooks:\n - "bash ./%s"\n' "$windows_pre_build_script" +fi +export WINDOWS_BEFORE_HOOKS_BLOCK + +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +envsubst < "$script_dir/../templates/.goreleaser-windows-template.yaml.tmpl" diff --git a/scripts/test-release-config-templates.py b/scripts/test-release-config-templates.py index b23e23e..6e3541e 100755 --- a/scripts/test-release-config-templates.py +++ b/scripts/test-release-config-templates.py @@ -52,6 +52,7 @@ def verify_case(go_main_package: str, brew_tap: str) -> None: "DOCKERFILE_LAMBDA_PATH": "Dockerfile.lambda", "PUBLIC_ECR_PUBLISH_TAG": "candidate", "EXTRA_FILES_BLOCK": "", + "WINDOWS_BEFORE_HOOKS_BLOCK": "", } rendered = {name: render(path, values) for name, path in TEMPLATES.items()} diff --git a/scripts/test-windows-pre-build-script.py b/scripts/test-windows-pre-build-script.py new file mode 100755 index 0000000..2293eba --- /dev/null +++ b/scripts/test-windows-pre-build-script.py @@ -0,0 +1,153 @@ +#!/usr/bin/env python3 +"""Exercise the release workflow's Windows renderer with real bash/envsubst/yq.""" + +from __future__ import annotations + +import os +import shlex +import subprocess +import sys +import tempfile +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +WORKFLOW = ROOT / ".github/workflows/release.yaml" +BASELINE = ROOT / "scripts/testdata/goreleaser-windows-default.yaml" +WINDOWS_JOB = '.jobs."goreleaser-windows"' +SCRIPT_PATH = "scripts/release-windows-resources.sh" + + +def query(expression: str, path: Path = WORKFLOW) -> str: + return subprocess.check_output(["yq", "-r", expression, str(path)], text=True).strip() + + +def step_field(name: str, field: str) -> str: + return query(f'{WINDOWS_JOB}.steps[] | select(.name == "{name}").{field}') + + +def verify_forwarding() -> None: + input_path = '.on.workflow_call.inputs.windows_pre_build_script' + assert query(f"{input_path}.type") == "string" + assert query(f"{input_path}.required") == "false" + assert query(f"{input_path}.default") == "" + assert step_field("Generate configs for Windows", "env.WINDOWS_PRE_BUILD_SCRIPT") == "${{ inputs.windows_pre_build_script }}" + assert step_field("Run GoReleaser for Windows", "env.RELEASE_TAG") == "${{ inputs.tag }}" + assert step_field("Run GoReleaser for Windows", "with.workdir") == "_caller" + assert step_field("Run GoReleaser for Windows", "with.args") == "release --clean --skip=publish --config ../_workflows/_generated/.goreleaser.windows.yaml" + # No pre-build opt-in or RELEASE_TAG forwarding on the other GoReleaser actions. + for job, name in ( + ("goreleaser-binaries", "Run GoReleaser"), + ("goreleaser-docker", "Run GoReleaser for Docker OCI"), + ("goreleaser-docker", "Run GoReleaser for Lambda"), + ): + assert query(f'.jobs."{job}".steps[] | select(.name == "{name}").env.RELEASE_TAG') == "null" + assert query(f'.jobs."{job}".steps[] | select(.name == "{name}").env.WINDOWS_PRE_BUILD_SCRIPT') == "null" + + +def verify_rendering() -> None: + render_command = step_field("Generate configs for Windows", "run") + validate_command = query('.jobs."validate-inputs".steps[] | select(.name == "Validate windows_pre_build_script has safe path").run') + assert query('.jobs."validate-inputs".steps[] | select(.name == "Validate windows_pre_build_script has safe path").env.WINDOWS_PRE_BUILD_SCRIPT') == "${{ inputs.windows_pre_build_script }}" + assert query('.jobs."validate-inputs".steps[] | select(.name == "Validate windows_pre_build_script has safe path").if') == "inputs.windows_pre_build_script != ''" + + with tempfile.TemporaryDirectory(prefix="windows-pre-build-") as temporary: + workspace = Path(temporary) + caller = workspace / "_caller" + workflows = workspace / "_workflows" + caller.mkdir() + workflows.mkdir() + (workflows / "_generated").mkdir() + (workflows / "scripts").symlink_to(ROOT / "scripts", target_is_directory=True) + (workflows / "templates").symlink_to(ROOT / "templates", target_is_directory=True) + generated = workflows / "_generated/.goreleaser.windows.yaml" + env = { + **os.environ, + "REPO_NAME": "bridge-client", + "GO_MAIN_PACKAGE": "./cmd/bridge-client", + "WXS_PATH": "app.wxs", + "WORKFLOWS_REF": "fixture-workflow-sha", + "RELEASE_TAG": "v1.2.3-rc.1+build.5", + "HOOK_EXIT_CODE": "0", + # The renderer must reset, not inherit, a previous hooks block. + "WINDOWS_BEFORE_HOOKS_BLOCK": "unexpected inherited hooks", + } + + def render(script: str) -> subprocess.CompletedProcess[bytes]: + return subprocess.run( + ["bash", "-c", render_command], + cwd=workflows, + env={**env, "WINDOWS_PRE_BUILD_SCRIPT": script}, + capture_output=True, + ) + + def validate(script: str) -> subprocess.CompletedProcess[bytes]: + return subprocess.run( + ["bash", "-c", validate_command], + env={**env, "WINDOWS_PRE_BUILD_SCRIPT": script}, + capture_output=True, + ) + + # Frozen, pre-hook rendering: compare bytes, including all newlines. + result = render("") + assert result.returncode == 0, result.stderr + assert generated.read_bytes() == BASELINE.read_bytes(), "empty input changed default Windows config bytes" + assert query(".before", generated) == "null" + + script = caller / SCRIPT_PATH + script.parent.mkdir() + script.write_text( + '#!/usr/bin/env bash\nset -euo pipefail\n' + 'printf "%s\\n" "$RELEASE_TAG" > generated-version\n' + 'exit "${HOOK_EXIT_CODE:-0}"\n' + ) + assert validate(SCRIPT_PATH).returncode == 0 + result = render(SCRIPT_PATH) + assert result.returncode == 0, result.stderr + block = f'before:\n hooks:\n - "bash ./{SCRIPT_PATH}"\n'.encode() + assert generated.read_bytes() == BASELINE.read_bytes().replace(b"builds:\n", block + b"builds:\n", 1) + hook = query(".before.hooks[0]", generated) + assert hook == f"bash ./{SCRIPT_PATH}" + marker = caller / "generated-version" + assert not marker.exists(), "rendering must not execute the caller script" + result = subprocess.run(shlex.split(hook), cwd=caller, env=env, capture_output=True) + assert result.returncode == 0, result.stderr + assert marker.read_text() == env["RELEASE_TAG"] + "\n" + result = subprocess.run(shlex.split(hook), cwd=caller, env={**env, "HOOK_EXIT_CODE": "17"}, capture_output=True) + assert result.returncode == 17, "hook command must preserve caller failure" + + # A missing script or directory fails before config generation. + for path in ("scripts/missing.sh", "scripts"): + assert validate(path).returncode == 0 + result = render(path) + assert result.returncode != 0, path + assert b"Windows pre-build script not found" in result.stderr + + for path in ( + "/absolute.sh", "../outside.sh", "scripts/../hook.sh", "scripts/foo..sh", + "C:/hook.sh", "scripts\\hook.sh", "scripts/a b.sh", "scripts/a\tb.sh", + "scripts/a\nb.sh", "scripts/hook.sh;echo", "$(touch injected)", + 'scripts/quote".sh', "scripts/{{template}}.sh", + ): + assert validate(path).returncode != 0, path + result = render(path) + assert result.returncode != 0, path + assert b"relative path with safe characters" in result.stderr + + # Existing ../_caller files must not cause a default consumer to opt in. + result = render("") + assert result.returncode == 0, result.stderr + assert generated.read_bytes() == BASELINE.read_bytes() + + +def main() -> int: + verify_forwarding() + verify_rendering() + return 0 + + +if __name__ == "__main__": + try: + raise SystemExit(main()) + except AssertionError as err: + print(f"test-windows-pre-build-script: {err}", file=sys.stderr) + raise SystemExit(1) diff --git a/scripts/testdata/goreleaser-windows-default.yaml b/scripts/testdata/goreleaser-windows-default.yaml new file mode 100644 index 0000000..6959f64 --- /dev/null +++ b/scripts/testdata/goreleaser-windows-default.yaml @@ -0,0 +1,75 @@ +## Windows template for zip and MSI installers (requires GoReleaser Pro for MSI) +## This runs on Windows runner with Wix Toolset +version: 2 +project_name: "bridge-client" +builds: + - binary: "bridge-client" + env: + - CGO_ENABLED=0 + id: windows + main: "./cmd/bridge-client" + goos: + - windows + goarch: + - amd64 +archives: + - id: windows-archive + builds: + - windows + format: zip + name_template: "{{ .ProjectName }}-v{{ .Version }}-{{ .Os }}-{{ .Arch }}" + files: + - none* +msi: + - id: windows-msi + ids: + - windows + wxs: app.wxs + mod_timestamp: "{{ .CommitTimestamp }}" + name: "{{ .ProjectName }}_v{{ .Version }}_windows_amd64" +release: + disable: true +checksum: + disable: true +sboms: + - id: sbom-archive + artifacts: archive + ids: + - windows-archive + - id: sbom-msi + artifacts: installer + ids: + - windows-msi +signs: + - id: cosign-archives + output: true + cmd: cosign + artifacts: archive + ids: + - windows-archive + certificate: "{{ .Env.artifact }}.cert" + args: + - "sign-blob" + - "--yes" + - "--output-signature={{ .Env.signature }}" + - "--output-certificate={{ .Env.certificate }}" + - "{{ .Env.artifact }}" + env: + - COSIGN_EXPERIMENTAL=1 + - id: cosign-msi + output: true + cmd: cosign + artifacts: installer + ids: + - windows-msi + certificate: "{{ .Env.artifact }}.cert" + args: + - "sign-blob" + - "--yes" + - "--output-signature={{ .Env.signature }}" + - "--output-certificate={{ .Env.certificate }}" + - "{{ .Env.artifact }}" + env: + - COSIGN_EXPERIMENTAL=1 +changelog: + disable: true diff --git a/templates/.goreleaser-windows-template.yaml.tmpl b/templates/.goreleaser-windows-template.yaml.tmpl index 3ffe581..0708015 100644 --- a/templates/.goreleaser-windows-template.yaml.tmpl +++ b/templates/.goreleaser-windows-template.yaml.tmpl @@ -2,7 +2,7 @@ ## This runs on Windows runner with Wix Toolset version: 2 project_name: "${REPO_NAME}" -builds: +${WINDOWS_BEFORE_HOOKS_BLOCK}builds: - binary: "${REPO_NAME}" env: - CGO_ENABLED=0 From bd5fa08cff7d8d07aa362530d0123d99f28c5b74 Mon Sep 17 00:00:00 2001 From: Steve Gontzes Date: Thu, 15 Oct 2026 14:17:13 +0000 Subject: [PATCH 2/2] refactor: reuse pinned safe-relative-path validation 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] --- .github/workflows/release.yaml | 94 ++++++++-------- Makefile | 1 + docs/release-workflow.md | 16 ++- scripts/render-goreleaser-windows-config.sh | 4 +- scripts/test-safe-relative-path.py | 113 ++++++++++++++++++++ scripts/test-windows-pre-build-script.py | 7 +- scripts/validate-safe-relative-path.sh | 13 +++ 7 files changed, 195 insertions(+), 53 deletions(-) create mode 100755 scripts/test-safe-relative-path.py create mode 100755 scripts/validate-safe-relative-path.sh diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index b6d657e..6821a44 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -149,57 +149,12 @@ jobs: exit 1 fi - - name: Validate dockerfile_template has safe path - if: inputs.dockerfile_template != '' - env: - DOCKERFILE_TEMPLATE: ${{ inputs.dockerfile_template }} - run: | - if [[ "$DOCKERFILE_TEMPLATE" == /* ]] || [[ "$DOCKERFILE_TEMPLATE" == *".."* ]] || [[ ! "$DOCKERFILE_TEMPLATE" =~ ^[A-Za-z0-9._/-]+$ ]]; then - echo "::error::dockerfile_template must be a relative path with safe characters and without '..' traversal. Got: $DOCKERFILE_TEMPLATE" - exit 1 - fi - - name: Validate docker_extra_files requires dockerfile_template if: inputs.docker_extra_files != '' && inputs.dockerfile_template == '' run: | echo "::error::docker_extra_files can only be used when dockerfile_template is set" exit 1 - - name: Validate docker_extra_files have safe paths - if: inputs.docker_extra_files != '' - env: - DOCKER_EXTRA_FILES: ${{ inputs.docker_extra_files }} - run: | - IFS=',' read -ra FILES <<< "$DOCKER_EXTRA_FILES" - for file in "${FILES[@]}"; do - file="${file#"${file%%[![:space:]]*}"}" - file="${file%"${file##*[![:space:]]}"}" - if [[ -z "$file" ]] || [[ "$file" == /* ]] || [[ "$file" == *".."* ]] || [[ ! "$file" =~ ^[A-Za-z0-9._/-]+$ ]]; then - echo "::error::docker_extra_files entries must be relative paths with safe characters and without '..' traversal. Got: $file" - exit 1 - fi - done - - - name: Validate msi_wxs_path has no path traversal - if: inputs.msi_wxs_path != '' - env: - WXS_PATH: ${{ inputs.msi_wxs_path }} - run: | - if [[ "$WXS_PATH" == /* ]] || [[ "$WXS_PATH" == *".."* ]] || [[ ! "$WXS_PATH" =~ ^[A-Za-z0-9._/-]+$ ]]; then - echo "::error::msi_wxs_path must be a relative path with safe characters and without '..' traversal. Got: $WXS_PATH" - exit 1 - fi - - - name: Validate windows_pre_build_script has safe path - if: inputs.windows_pre_build_script != '' - env: - WINDOWS_PRE_BUILD_SCRIPT: ${{ inputs.windows_pre_build_script }} - run: | - if [[ "$WINDOWS_PRE_BUILD_SCRIPT" == /* ]] || [[ "$WINDOWS_PRE_BUILD_SCRIPT" == *".."* ]] || [[ ! "$WINDOWS_PRE_BUILD_SCRIPT" =~ ^[A-Za-z0-9._/-]+$ ]]; then - echo "::error::windows_pre_build_script must be a relative path with safe characters and without '..' traversal. Got: $WINDOWS_PRE_BUILD_SCRIPT" - exit 1 - fi - - name: Validate GORELEASER_PRO_KEY when msi enabled if: inputs.msi == true env: @@ -237,6 +192,55 @@ jobs: ref: ${{ steps.workflow-version.outputs.sha }} persist-credentials: false + - name: Validate dockerfile_template has safe path + if: inputs.dockerfile_template != '' + working-directory: _workflows + env: + DOCKERFILE_TEMPLATE: ${{ inputs.dockerfile_template }} + run: | + if ! bash ./scripts/validate-safe-relative-path.sh "$DOCKERFILE_TEMPLATE"; then + echo "::error::dockerfile_template must be a relative path with safe characters and without '..' traversal. Got: $DOCKERFILE_TEMPLATE" + exit 1 + fi + + - name: Validate docker_extra_files have safe paths + if: inputs.docker_extra_files != '' + working-directory: _workflows + env: + DOCKER_EXTRA_FILES: ${{ inputs.docker_extra_files }} + run: | + IFS=',' read -ra FILES <<< "$DOCKER_EXTRA_FILES" + for file in "${FILES[@]}"; do + file="${file#"${file%%[![:space:]]*}"}" + file="${file%"${file##*[![:space:]]}"}" + if ! bash ./scripts/validate-safe-relative-path.sh "$file"; then + echo "::error::docker_extra_files entries must be relative paths with safe characters and without '..' traversal. Got: $file" + exit 1 + fi + done + + - name: Validate msi_wxs_path has no path traversal + if: inputs.msi_wxs_path != '' + working-directory: _workflows + env: + WXS_PATH: ${{ inputs.msi_wxs_path }} + run: | + if ! bash ./scripts/validate-safe-relative-path.sh "$WXS_PATH"; then + echo "::error::msi_wxs_path must be a relative path with safe characters and without '..' traversal. Got: $WXS_PATH" + exit 1 + fi + + - name: Validate windows_pre_build_script has safe path + if: inputs.windows_pre_build_script != '' + working-directory: _workflows + env: + WINDOWS_PRE_BUILD_SCRIPT: ${{ inputs.windows_pre_build_script }} + run: | + if ! bash ./scripts/validate-safe-relative-path.sh "$WINDOWS_PRE_BUILD_SCRIPT"; then + echo "::error::windows_pre_build_script must be a relative path with safe characters and without '..' traversal. Got: $WINDOWS_PRE_BUILD_SCRIPT" + exit 1 + fi + - name: Normalize release options id: release-options working-directory: _workflows diff --git a/Makefile b/Makefile index 777242c..2ed6b21 100644 --- a/Makefile +++ b/Makefile @@ -22,6 +22,7 @@ test-scripts: bash scripts/test-normalize-release-options.sh python3 scripts/test-release-config-templates.py python3 scripts/test-windows-pre-build-script.py + python3 scripts/test-safe-relative-path.py bash scripts/test-release-workflow-tag-pin.sh bash scripts/test-s3-release-uploads.sh bash scripts/test-validate-release-artifacts.sh diff --git a/docs/release-workflow.md b/docs/release-workflow.md index 74373e0..2897e6d 100644 --- a/docs/release-workflow.md +++ b/docs/release-workflow.md @@ -27,15 +27,21 @@ Validates workflow inputs before proceeding: `^[a-z][a-z0-9-]{0,99}$` - Ensures `dockerfile_template` is only used when `lambda: false` - Ensures `docker_extra_files` is only used when `dockerfile_template` is set -- Ensures `msi_wxs_path` has no path traversal (`..` or absolute paths) -- Ensures `windows_pre_build_script` is a safe relative path without absolute - paths, `..` traversal, whitespace, or shell metacharacters - Ensures `GORELEASER_PRO_KEY` is provided when `msi: true` ### determine-workflows-ref Resolves the exact SHA of the shared workflow, then uses the pinned workflow source to normalize `go_main_package` and `brew_tap`. An omitted main package becomes `./cmd/`; a root package uses `./`. The workflow rejects non-relative package paths and tap values containing a path separator. The pinned reference is embedded in all provenance attestations, ensuring verifiability. +Before option normalization, the pinned checkout validates `dockerfile_template`, +`msi_wxs_path`, `windows_pre_build_script`, and trimmed `docker_extra_files` +entries through `scripts/validate-safe-relative-path.sh`. The helper has the +single safe-path predicate; callers retain their existing diagnostics and +empty-input guards. +These guarded checks run after the existing pinned checkout rather than adding +an unpinned fetch or permissions to `validate-inputs`; all builds already depend +on `determine-workflows-ref`. No job dependencies or permissions change. + ### release-preflight Runs opted-in source integrity checks against the exact tagged caller source before any build job: @@ -318,6 +324,10 @@ The fixture hook also checks the caller working directory, release-tag forwarding, and command failure propagation without compiling or publishing. These are local fixture checks, not a deployed Windows release proof. +`python3 scripts/test-safe-relative-path.py` additionally compares the shared +predicate and every workflow/render call site's exit status and diagnostic bytes +against the previous Bash rules, including extra-file trimming and empty guards. + ### Validation Script The `scripts/validate-release-artifacts.sh` script validates: diff --git a/scripts/render-goreleaser-windows-config.sh b/scripts/render-goreleaser-windows-config.sh index 70f5c90..faf7c91 100755 --- a/scripts/render-goreleaser-windows-config.sh +++ b/scripts/render-goreleaser-windows-config.sh @@ -9,9 +9,10 @@ fi windows_pre_build_script="$1" caller_root="$2" WINDOWS_BEFORE_HOOKS_BLOCK="" +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" if [ -n "$windows_pre_build_script" ]; then - if [[ "$windows_pre_build_script" == /* ]] || [[ "$windows_pre_build_script" == *".."* ]] || [[ ! "$windows_pre_build_script" =~ ^[A-Za-z0-9._/-]+$ ]]; then + if ! bash "$script_dir/validate-safe-relative-path.sh" "$windows_pre_build_script"; then echo "::error::windows_pre_build_script must be a relative path with safe characters and without '..' traversal. Got: $windows_pre_build_script" >&2 exit 1 fi @@ -23,5 +24,4 @@ if [ -n "$windows_pre_build_script" ]; then fi export WINDOWS_BEFORE_HOOKS_BLOCK -script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" envsubst < "$script_dir/../templates/.goreleaser-windows-template.yaml.tmpl" diff --git a/scripts/test-safe-relative-path.py b/scripts/test-safe-relative-path.py new file mode 100755 index 0000000..255d190 --- /dev/null +++ b/scripts/test-safe-relative-path.py @@ -0,0 +1,113 @@ +#!/usr/bin/env python3 +"""Compare the consolidated path checks with the pre-refactor Bash behavior.""" + +import os +from pathlib import Path +import subprocess +import tempfile + +ROOT = Path(__file__).resolve().parent.parent +WORKFLOW = ROOT / ".github/workflows/release.yaml" +JOB = '.jobs."determine-workflows-ref"' +HELPER = ROOT / "scripts/validate-safe-relative-path.sh" +# Frozen pre-refactor predicate, intentionally independent of the production helper. +LEGACY = r'''if [[ "$1" == /* ]] || [[ "$1" == *".."* ]] || [[ ! "$1" =~ ^[A-Za-z0-9._/-]+$ ]]; then exit 1; fi''' +CASES = ( + "", "a", "A-Z_09/file.sh", "-script.sh", ".", "./script.sh", "scripts/", "a//b", + "/", "/absolute.sh", "//server/share", "..", "../file", "a/../file", "a..b", + "C:/file", r"scripts\file.sh", "a b", "a\tb", "a\nb", "a\rb", "a;b", "a&b", + "$(touch injected)", "a|b", "a'b", 'a"b', "a`b", "a*b", "a?b", "a[b", "a:b", + "a=b", "a#b", "a%b", "a+b", "café.sh", "日本語.sh", "scripts/hook.sh", +) + + +def query(expression): + return subprocess.check_output(["yq", "-r", expression, str(WORKFLOW)], text=True).strip() + + +def run(args, env=None, cwd=ROOT): + result = subprocess.run(args, env={**os.environ, **(env or {})}, cwd=cwd, capture_output=True) + return result.returncode, result.stdout, result.stderr + + +def step(name, field): + return query(f'{JOB}.steps[] | select(.name == "{name}").{field}') + + +def test_predicate_and_workflow_sites(): + names = query(f"{JOB}.steps[].name").splitlines() + checkout = names.index("Checkout pinned connector workflows") + normalize = names.index("Normalize release options") + assert query(f"{JOB}.needs") == "validate-inputs" + assert query(f"{JOB}.permissions.actions") == "read" + assert query(f"{JOB}.permissions.contents") == "read" + assert query(f'{JOB}.steps[] | select(.name == "Checkout pinned connector workflows").with.ref') == "${{ steps.workflow-version.outputs.sha }}" + sites = ( + ("Validate dockerfile_template has safe path", "DOCKERFILE_TEMPLATE", "dockerfile_template"), + ("Validate msi_wxs_path has no path traversal", "WXS_PATH", "msi_wxs_path"), + ("Validate windows_pre_build_script has safe path", "WINDOWS_PRE_BUILD_SCRIPT", "windows_pre_build_script"), + ) + for name, variable, input_name in sites: + assert checkout < names.index(name) < normalize + assert step(name, '"working-directory"') == "_workflows" + assert step(name, "if") == f"inputs.{input_name} != ''" + assert step(name, f"env.{variable}") == "${{ inputs." + input_name + " }}" + command = step(name, "run") + for candidate in CASES: + legacy = run(["bash", "-c", LEGACY, "legacy", candidate]) + assert run(["bash", str(HELPER), candidate]) == legacy, repr(candidate) + invalid = legacy[0] != 0 + message = (f"::error::{input_name} must be a relative path with safe characters " + f"and without '..' traversal. Got: {candidate}\n").encode() + expected = (1, message, b"") if invalid else (0, b"", b"") + assert run(["bash", "-c", command], {variable: candidate}) == expected, (name, repr(candidate)) + + +def test_extra_files_preserves_trimming_and_diagnostics(): + name = "Validate docker_extra_files have safe paths" + assert step(name, '"working-directory"') == "_workflows" + assert step(name, "if") == "inputs.docker_extra_files != ''" + command = step(name, "run") + legacy = r'''IFS=',' read -ra FILES <<< "$DOCKER_EXTRA_FILES" +for file in "${FILES[@]}"; do + file="${file#"${file%%[![:space:]]*}"}" + file="${file%"${file##*[![:space:]]}"}" + if [[ -z "$file" ]] || [[ "$file" == /* ]] || [[ "$file" == *".."* ]] || [[ ! "$file" =~ ^[A-Za-z0-9._/-]+$ ]]; then + echo "::error::docker_extra_files entries must be relative paths with safe characters and without '..' traversal. Got: $file" + exit 1 + fi +done''' + values = CASES + (" a , b ", "a,,b", "a,b,", ",a", " ", "a, /bad", "a, ../bad", "a,\tb\t") + for candidate in values: + env = {"DOCKER_EXTRA_FILES": candidate} + assert run(["bash", "-c", command], env) == run(["bash", "-c", legacy], env), repr(candidate) + + +def test_renderer_preserves_error_streams_and_default_bytes(): + baseline = (ROOT / "scripts/testdata/goreleaser-windows-default.yaml").read_bytes() + renderer = ROOT / "scripts/render-goreleaser-windows-config.sh" + with tempfile.TemporaryDirectory(prefix="safe-path-render-") as temporary: + caller = Path(temporary) + (caller / "scripts").mkdir() + (caller / "scripts/hook.sh").write_text("exit 0\n") + env = {"REPO_NAME": "bridge-client", "GO_MAIN_PACKAGE": "./cmd/bridge-client", "WXS_PATH": "app.wxs"} + for candidate in CASES: + actual = run(["bash", str(renderer), candidate, str(caller)], env) + if not candidate: + expected = (0, baseline, b"") + elif run(["bash", "-c", LEGACY, "legacy", candidate])[0]: + expected = (1, b"", ("::error::windows_pre_build_script must be a relative path with safe characters " + f"and without '..' traversal. Got: {candidate}\n").encode()) + elif not (caller / candidate).is_file(): + expected = (1, b"", f"::error::Windows pre-build script not found: {candidate}\n".encode()) + else: + block = f'before:\n hooks:\n - "bash ./{candidate}"\n'.encode() + expected = (0, baseline.replace(b"builds:\n", block + b"builds:\n", 1), b"") + assert actual == expected, repr(candidate) + + +if __name__ == "__main__": + test_predicate_and_workflow_sites() + test_extra_files_preserves_trimming_and_diagnostics() + test_renderer_preserves_error_streams_and_default_bytes() + print("shared path predicates, caller diagnostics, and default config bytes unchanged") diff --git a/scripts/test-windows-pre-build-script.py b/scripts/test-windows-pre-build-script.py index 2293eba..a296b7f 100755 --- a/scripts/test-windows-pre-build-script.py +++ b/scripts/test-windows-pre-build-script.py @@ -46,9 +46,9 @@ def verify_forwarding() -> None: def verify_rendering() -> None: render_command = step_field("Generate configs for Windows", "run") - validate_command = query('.jobs."validate-inputs".steps[] | select(.name == "Validate windows_pre_build_script has safe path").run') - assert query('.jobs."validate-inputs".steps[] | select(.name == "Validate windows_pre_build_script has safe path").env.WINDOWS_PRE_BUILD_SCRIPT') == "${{ inputs.windows_pre_build_script }}" - assert query('.jobs."validate-inputs".steps[] | select(.name == "Validate windows_pre_build_script has safe path").if') == "inputs.windows_pre_build_script != ''" + validate_command = query('.jobs."determine-workflows-ref".steps[] | select(.name == "Validate windows_pre_build_script has safe path").run') + assert query('.jobs."determine-workflows-ref".steps[] | select(.name == "Validate windows_pre_build_script has safe path").env.WINDOWS_PRE_BUILD_SCRIPT') == "${{ inputs.windows_pre_build_script }}" + assert query('.jobs."determine-workflows-ref".steps[] | select(.name == "Validate windows_pre_build_script has safe path").if') == "inputs.windows_pre_build_script != ''" with tempfile.TemporaryDirectory(prefix="windows-pre-build-") as temporary: workspace = Path(temporary) @@ -83,6 +83,7 @@ def render(script: str) -> subprocess.CompletedProcess[bytes]: def validate(script: str) -> subprocess.CompletedProcess[bytes]: return subprocess.run( ["bash", "-c", validate_command], + cwd=workflows, env={**env, "WINDOWS_PRE_BUILD_SCRIPT": script}, capture_output=True, ) diff --git a/scripts/validate-safe-relative-path.sh b/scripts/validate-safe-relative-path.sh new file mode 100755 index 0000000..1c0eba1 --- /dev/null +++ b/scripts/validate-safe-relative-path.sh @@ -0,0 +1,13 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Predicate only: callers retain their existing diagnostics and output streams. +if [ "$#" -ne 1 ]; then + echo "usage: $0 " >&2 + exit 2 +fi + +path="$1" +if [[ "$path" == /* ]] || [[ "$path" == *".."* ]] || [[ ! "$path" =~ ^[A-Za-z0-9._/-]+$ ]]; then + exit 1 +fi