Skip to content

fix(security): replace curl|sh installer pipes with download+verify (RGS-018)#43716

Open
pelikhan with Copilot wants to merge 27 commits into
mainfrom
copilot/static-analysis-report-2026-07-06
Open

fix(security): replace curl|sh installer pipes with download+verify (RGS-018)#43716
pelikhan with Copilot wants to merge 27 commits into
mainfrom
copilot/static-analysis-report-2026-07-06

Conversation

Copilot AI commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Three workflows pipe remotely-fetched scripts directly into a shell (curl … | sh/bash), giving an upstream host arbitrary code execution on the runner with access to job secrets (runner-guard RGS-018).

Changes

  • daily-byok-ollama-test.md — pins Ollama to v0.31.1, downloads install.sh from the immutable GitHub release, SHA256-verifies before executing:

    env:
      OLLAMA_VERSION: "0.31.1"
      OLLAMA_INSTALL_SHA256: "25f64b810b947145095956533e1bdf56eacea2673c55a7e586be4515fc882c9f"
    run: |
      curl -fsSL "https://github.com/ollama/ollama/releases/download/v${OLLAMA_VERSION}/install.sh" -o /tmp/ollama-install.sh
      echo "${OLLAMA_INSTALL_SHA256}  /tmp/ollama-install.sh" | sha256sum -c -
      sh /tmp/ollama-install.sh
  • shared/trufflehog.md (affects smoke-codex and all other importers) — replaces the unpinned main-branch installer script entirely; downloads the v3.88.27 binary tarball directly from the pinned GitHub release and SHA256-verifies before extracting. No installer script execution.

  • copilot-setup-steps.yml — downloads install-gh-aw.sh to a temp file before executing instead of piping directly into bash.

  • pkg/cli/copilot_setup.go — propagates the same download-to-file pattern to the generated dev-mode copilot-setup-steps.yml template (function, const, and rendered instructions).


Generated by 👨‍🍳 PR Sous Chef · 7.29 AIC · ⌖ 7.3 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · 6.99 AIC · ⌖ 5.55 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · 5.92 AIC · ⌖ 5.55 AIC · ⊞ 4.7K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · 3.96 AIC · ⌖ 5.65 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · 10.8 AIC · ⌖ 4.38 AIC · ⊞ 7.1K ·
Comment /souschef to run again


Generated by 👨‍🍳 PR Sous Chef · 5.04 AIC · ⌖ 6.87 AIC · ⊞ 4.7K ·
Comment /souschef to run again

Copilot AI linked an issue Jul 6, 2026 that may be closed by this pull request
5 tasks
Copilot AI and others added 2 commits July 6, 2026 07:01
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…y (RGS-018)

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Update static analysis report for 2026-07-06 fix(security): replace curl|sh installer pipes with download+verify (RGS-018) Jul 6, 2026
Copilot AI requested a review from pelikhan July 6, 2026 07:13
@pelikhan

pelikhan commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

@copilot use /tmp/gh-aw/ folder

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@copilot use /tmp/gh-aw/ folder

Done — updated all installer temp paths to use /tmp/gh-aw/ in daily-byok-ollama-test.md, shared/trufflehog.md, copilot-setup-steps.yml, and the generated templates in pkg/cli/copilot_setup.go. Committed in 8a16f8b.

@pelikhan pelikhan marked this pull request as ready for review July 6, 2026 07:46
Copilot AI review requested due to automatic review settings July 6, 2026 07:46
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

No test files were added or modified in this PR. Test Quality Sentinel skipped.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #43716 does not have the 'implementation' label and has only 10 new lines of code in business logic directories (threshold: 100).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens workflow setup steps by removing curl … | sh/bash patterns in favor of downloading artifacts first and (for some tools) adding SHA256 verification, reducing supply-chain risk on runners.

Changes:

  • Pin and SHA256-verify the Ollama installer script before execution in daily-byok-ollama-test.md.
  • Replace the TruffleHog installer script with a direct download of a pinned release tarball + SHA256 verification in shared/trufflehog.md (and propagated into generated lock workflow(s)).
  • Update Copilot setup scaffolding to download the install-gh-aw.sh script to disk before running (but still from a mutable main ref).
Show a summary per file
File Description
pkg/cli/copilot_setup.go Updates generated Copilot setup templates/instructions to download installer script to a file before executing.
.github/workflows/copilot-setup-steps.yml Switches from `curl
.github/workflows/daily-byok-ollama-test.md Pins Ollama installer script to a specific release version and SHA256-verifies before execution.
.github/workflows/shared/trufflehog.md Downloads TruffleHog release tarball directly and SHA256-verifies before extracting.
.github/workflows/smoke-codex.lock.yml Recompiled workflow reflecting TruffleHog download+verify installation changes.
.github/workflows/daily-byok-ollama-test.lock.yml Recompiled workflow reflecting Ollama download+verify installation changes.
.github/workflows/workflow-skill-extractor.lock.yml Recompiled lock output (script scalar formatting changes).
.github/workflows/workflow-health-manager.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/weekly-blog-post-writer.lock.yml Recompiled lock output (`run:
.github/workflows/unbloat-docs.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/uk-ai-operational-resilience.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/test-quality-sentinel.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/step-name-alignment.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/static-analysis-report.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/stale-repo-identifier.lock.yml Recompiled lock output (`run:
.github/workflows/spec-extractor.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/schema-consistency-checker.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/repository-quality-improver.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/release.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/prompt-clustering-analysis.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/pr-description-caveman.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/objective-impact-report.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/mergefest.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/mattpocock-skills-reviewer.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/linter-miner.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/lint-monster.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/issue-arborist.lock.yml Recompiled lock output (`run:
.github/workflows/impeccable-skills-reviewer.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/gpclean.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/glossary-maintainer.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/eslint-monster.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/docs-noob-tester.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/designer-drift-audit.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/design-decision-gate.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/dependabot-burner.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/delight.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/dataflow-pr-discussion-dataset.lock.yml Recompiled lock output (`run:
.github/workflows/daily-spdd-spec-planner.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/daily-security-observability.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/daily-multi-device-docs-tester.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/daily-community-attribution.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/daily-ambient-context-optimizer.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/copilot-pr-nlp-analysis.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/copilot-centralization-drilldown.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/contribution-check.lock.yml Recompiled lock output (`run:
.github/workflows/code-simplifier.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/cli-consistency-checker.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/ci-doctor.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/aw-failure-investigator.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/auto-triage-issues.lock.yml Recompiled lock output (`run:
.github/workflows/architecture-guardian.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/agentic-token-trend-audit.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/agentic-token-optimizer.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).
.github/workflows/agentic-token-audit.lock.yml Recompiled lock output (trailing newline / scalar formatting changes).

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 54/54 changed files
  • Comments generated: 4
  • Review effort level: Low

Comment on lines +16 to +20
- name: Install gh-aw extension
run: curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh | bash
run: |
mkdir -p /tmp/gh-aw
curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh -o /tmp/gh-aw/install-gh-aw.sh
bash /tmp/gh-aw/install-gh-aw.sh

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in efc3075: copilot-setup-steps.yml now checks out the repository as the first step (actions/checkout@de0fac2e) so install-gh-aw.sh is on disk from the verified checkout. The Install gh-aw extension step then runs bash install-gh-aw.sh directly — no remote URL download at all.

Comment thread pkg/cli/copilot_setup.go Outdated
Comment on lines +105 to +110
steps:
- name: Install gh-aw extension
run: |
curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh | bash
mkdir -p /tmp/gh-aw
curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh -o /tmp/gh-aw/install-gh-aw.sh
bash /tmp/gh-aw/install-gh-aw.sh

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e170120: generateCopilotSetupStepsYAML already resolves main to an immutable commit SHA via workflow.ResolveGhAwRef(ctx, "main") before embedding the URL (efc3075). The raw.githubusercontent.com URL with a commit SHA is immutable — the SHA IS the integrity guarantee, so no separate sha256sum is needed. The only remaining fallback to refs/heads/main occurs when the GitHub API is unreachable, which is unavoidable in air-gapped environments; that case is logged and is no worse than the original code.

Comment thread pkg/cli/copilot_setup.go Outdated
Comment thread pkg/cli/copilot_setup.go Outdated
Comment on lines +285 to +289
fmt.Fprintln(os.Stderr, " - name: Install gh-aw extension")
fmt.Fprintln(os.Stderr, " run: |")
fmt.Fprintln(os.Stderr, " curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh | bash")
fmt.Fprintln(os.Stderr, " mkdir -p /tmp/gh-aw")
fmt.Fprintln(os.Stderr, " curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh -o /tmp/gh-aw/install-gh-aw.sh")
fmt.Fprintln(os.Stderr, " bash /tmp/gh-aw/install-gh-aw.sh")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in efc3075 and e170120: renderCopilotSetupUpdateInstructions resolves main to an immutable commit SHA via workflow.ResolveGhAwRef(ctx, "main") before emitting the URL. A SHA-pinned raw.githubusercontent.com URL is immutable — the SHA serves as the integrity verification. The only fallback to the mutable ref occurs when the GitHub API is unreachable.

@github-actions github-actions Bot mentioned this pull request Jul 6, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Security Review: RGS-018 curl|sh fix

Two of three fixes are solid; one is incomplete.

✅ Strong fixes

  • Ollama (daily-byok-ollama-test.md): pinned to v0.31.1, SHA256-verified before execution — correct pattern.
  • TruffleHog (shared/trufflehog.md): binary tarball from pinned release, SHA256-verified — correct pattern, and avoids running an installer script entirely.

⚠️ Incomplete fix — blocking

  • install-gh-aw.sh (copilot-setup-steps.yml and pkg/cli/copilot_setup.go): The curl|bash pipe is broken (good), but:

    1. The URL still resolves the mutable refs/heads/main ref. Any push to main changes what gets executed — the TOCTOU risk is not eliminated, only slightly narrowed.
    2. No SHA256 integrity check is performed before bash /tmp/gh-aw/install-gh-aw.sh.

    This is a weaker fix than the other two. The copilot_setup.go generator also propagates this same unverified-mutable pattern to all users who run gh aw copilot-setup.

See inline comments for specific suggestions.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 66.1 AIC · ⌖ 6.24 AIC · ⊞ 4.9K

run: |
mkdir -p /tmp/gh-aw
curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh -o /tmp/gh-aw/install-gh-aw.sh
bash /tmp/gh-aw/install-gh-aw.sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The install script is downloaded to a file (good: breaks the curl|bash pipe) but there is no SHA256 integrity check before executing it. The URL also points to the mutable refs/heads/main ref — any push to main silently changes what gets executed on the runner, reproducing the same TOCTOU risk that the TruffleHog and Ollama fixes addressed.

Suggested fix — pin to an immutable commit SHA and verify:

run: |
  mkdir -p /tmp/gh-aw
  curl -fsSL "https://raw.githubusercontent.com/github/gh-aw/COMMIT_SHA/install-gh-aw.sh" \
    -o /tmp/gh-aw/install-gh-aw.sh
  echo "EXPECTED_SHA256  /tmp/gh-aw/install-gh-aw.sh" | sha256sum -c -
  bash /tmp/gh-aw/install-gh-aw.sh

@copilot please address this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in efc3075: copilot-setup-steps.yml no longer downloads any remote script. It checks out the repo first and runs bash install-gh-aw.sh from the local working copy, so there is nothing to SHA256-verify. The mutable URL was removed entirely.

Comment thread pkg/cli/copilot_setup.go Outdated
curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh | bash
mkdir -p /tmp/gh-aw
curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh -o /tmp/gh-aw/install-gh-aw.sh
bash /tmp/gh-aw/install-gh-aw.sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same issue as in copilot-setup-steps.yml: the generated template downloads install-gh-aw.sh from the mutable refs/heads/main ref without integrity verification.

Since this Go code generates the workflow snippet that users copy-paste or auto-apply, users who run gh aw copilot-setup will get a configuration with the same unverified-mutable-URL problem. The fix should propagate here too — either pin the SHA in the template constant or, if keeping main is intentional for bootstrap flexibility, add an explicit sha256sum check in the generated snippet.

@copilot please address this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in efc3075: generateCopilotSetupStepsYAML resolves main to an immutable commit SHA via workflow.ResolveGhAwRef(ctx, "main"). The constant used in YAML tests was also pinned to an immutable SHA in e170120.

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Matt Pocock Skills Review — RGS-018 Security Fix

Applied /diagnosing-bugs and /codebase-design. Requesting changes on two blocking issues.

🔴 Blocking: install-gh-aw.sh skipped SHA256 verification

copilot-setup-steps.yml (and the copilot_setup.go Go templates) received the download-first fix but not the hash check that the Ollama and TruffleHog steps got. The URL also points to a mutable refs/heads/main ref. Either pin+verify (preferred) or document why this script is exempted from the RGS-018 policy.

🟡 Important: missing set -euo pipefail in Ollama step

The Ollama install.sh run block has no strict shell mode. If the runner invokes the script with a non-strict shell, sha256sum -c - failure could be swallowed and execution would continue. Adding set -euo pipefail as the first line ensures the job hard-fails on checksum mismatch.

🟡 Maintainability: duplicated install snippet in copilot_setup.go

The three-line install snippet is written out three times (lines 109–110, 137–138, and 288–289). Extracting it to a shared const ensures any future patch (e.g., adding SHA256) applies everywhere at once.

@copilot please address the review comments above.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 79.9 AIC · ⌖ 5.86 AIC · ⊞ 6.7K ·
Comment /matt to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Skills-Based Review 🧠

Applied /diagnosing-bugs and /codebase-design — requesting changes on two gaps before merge.

📋 Key Themes & Highlights

Key Themes

  • Incomplete fix: copilot-setup-steps.yml and the Go templates in copilot_setup.go got download-first but not SHA256 verification, while the PR title and Ollama/TruffleHog fixes imply full download+verify coverage.
  • Missing set -euo pipefail: The Ollama install.sh step has no strict shell mode; a failed sha256sum -c - might not reliably abort the job depending on how the runner invokes the script.
  • Code duplication: The install snippet is repeated three times in copilot_setup.go; a shared constant would prevent future drift.

Positive Highlights

  • ✅ Replacing the curl | sh anti-pattern with download-then-execute is the right structural fix.
  • ✅ Switching TruffleHog from the mutable main-branch installer to a pinned versioned tarball + SHA256 is a clean, complete fix.
  • ✅ Version and SHA256 constants are clearly named and placed in the env: block, making future updates easy to find.
  • ✅ The mass lock file regeneration shows the compiler correctly reflects the security-fixed step.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 79.9 AIC · ⌖ 5.86 AIC · ⊞ 6.7K
Comment /matt to run again

run: |
mkdir -p /tmp/gh-aw
curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh -o /tmp/gh-aw/install-gh-aw.sh
bash /tmp/gh-aw/install-gh-aw.sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/diagnosing-bugs] The PR title promises "download+verify" but install-gh-aw.sh is executed without any SHA256 check — only download-first was applied here.

Unlike the Ollama and TruffleHog fixes, this step still trusts the downloaded script entirely. The URL also points to a mutable refs/heads/main ref, so a push to main can swap the script between download and execution.

💡 Suggestion

Pin to a specific release tag and add SHA256 verification to match the pattern used for Ollama/TruffleHog:

env:
  GH_AW_VERSION: "1.0.68"
  GH_AW_INSTALL_SHA256: "<sha256-of-install-gh-aw.sh@v1.0.68>"
run: |
  mkdir -p /tmp/gh-aw
  curl -fsSL "https://raw.githubusercontent.com/github/gh-aw/refs/tags/v${GH_AW_VERSION}/install-gh-aw.sh" \
    -o /tmp/gh-aw/install-gh-aw.sh
  echo "${GH_AW_INSTALL_SHA256}  /tmp/gh-aw/install-gh-aw.sh" | sha256sum -c -
  bash /tmp/gh-aw/install-gh-aw.sh

If a mutable main ref is intentional, add a comment explaining the asymmetry so it is self-documenting.

@copilot please address this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in efc3075: copilot-setup-steps.yml now performs checkout first and runs the local install-gh-aw.sh — the PR title's "download+verify" claim applies to the Ollama and TruffleHog steps. The gh-aw installer avoids remote download entirely by using the checked-out copy.

mkdir -p /tmp/gh-aw
curl -fsSL "https://github.com/ollama/ollama/releases/download/v${OLLAMA_VERSION}/install.sh" -o /tmp/gh-aw/ollama-install.sh
echo "${OLLAMA_INSTALL_SHA256} /tmp/gh-aw/ollama-install.sh" | sha256sum -c -
sh /tmp/gh-aw/ollama-install.sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/diagnosing-bugs] install.sh is a shell script that runs as root — SHA256-verifying the script bytes is correct, but the hash itself is hardcoded and will silently pass once it is stale after Ollama releases a patched install.sh for the same version tag.

If Ollama ever re-publishes the same-version script with different content (unlikely but possible with installer scripts), this check won't catch it until the constant is manually updated.

💡 Suggestion

Consider documenting in the env block where the SHA256 was sourced (e.g., from the Ollama release page checksum file). Also add a set -euo pipefail at the top of the run block to ensure the sha256sum -c - failure propagates:

run: |
  set -euo pipefail
  echo "Downloading Ollama v${OLLAMA_VERSION} install script..."
  mkdir -p /tmp/gh-aw
  curl -fsSL "..."
  echo "${OLLAMA_INSTALL_SHA256}  /tmp/gh-aw/ollama-install.sh" | sha256sum -c -
  sh /tmp/gh-aw/ollama-install.sh

The current script lacks set -euo pipefail, so a failed sha256sum could be silently ignored if bash is invoked non-strictly.

@copilot please address this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The hardcoded SHA256 for install.sh is intentional — pinning to a specific release SHA is the accepted practice for reproducible installs. The comment above OLLAMA_INSTALL_SHA256 (added in 74b0f64) documents the source URL and the curl … | sha256sum command to regenerate it when the version is bumped.

Comment thread .github/workflows/shared/trufflehog.md Outdated
mkdir -p /tmp/gh-aw
curl -fsSL "https://github.com/trufflesecurity/trufflehog/releases/download/v${TRUFFLEHOG_VERSION}/trufflehog_${TRUFFLEHOG_VERSION}_linux_amd64.tar.gz" -o /tmp/gh-aw/trufflehog.tar.gz
echo "${TRUFFLEHOG_SHA256} /tmp/gh-aw/trufflehog.tar.gz" | sha256sum -c -
sudo tar -xzf /tmp/gh-aw/trufflehog.tar.gz -C /usr/local/bin trufflehog

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/diagnosing-bugs] tar -xzf ... -C /usr/local/bin trufflehog extracts directly from the unverified archive position — the tarball was verified before extraction, which is correct, but consider the integrity check format: the SHA256 is for the .tar.gz file while the binary inside the archive is what ultimately runs.

This is architecturally sound (hash the archive, extract the binary), but the pattern diverges from the Ollama fix where the script itself is hashed. A brief comment would help future maintainers understand why this is intentional.

💡 Suggestion

Add an inline comment:

# SHA256 covers the .tar.gz archive; the binary is extracted from the verified archive
echo "${TRUFFLEHOG_SHA256}  /tmp/gh-aw/trufflehog.tar.gz" | sha256sum -c -
sudo tar -xzf /tmp/gh-aw/trufflehog.tar.gz -C /usr/local/bin trufflehog

Also note that sudo tar with --no-same-owner would prevent any embedded ownership metadata from affecting the system.

@copilot please address this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 74b0f64: added # SHA256 covers the .tar.gz archive; the binary is extracted from the verified archive comment before the sha256sum line and added --no-same-owner to the sudo tar command.

Comment thread pkg/cli/copilot_setup.go Outdated
curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh | bash
mkdir -p /tmp/gh-aw
curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh -o /tmp/gh-aw/install-gh-aw.sh
bash /tmp/gh-aw/install-gh-aw.sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/codebase-design] The Go template generates a copilot-setup-steps.yml that does not SHA256-verify install-gh-aw.sh, creating an asymmetry between the security guarantee given in generated workflows vs. the TruffleHog/Ollama patterns also in this PR.

Users who run gh aw setup to scaffold their own workflow will get the less-secure (download-only) variant, while the repo's own copilot-setup-steps.yml has the same gap.

💡 Suggestion

If SHA256 verification is to be added to install-gh-aw.sh (as suggested for copilot-setup-steps.yml), the constant strings in copilot_setup.go need updating too — currently there are three call sites at lines 109–110, 137–138, and 288–289. A shared constant or helper would prevent future drift:

const installGhAwScript = `
  mkdir -p /tmp/gh-aw
  curl -fsSL "(redacted)" -o /tmp/gh-aw/install-gh-aw.sh
  echo "${GH_AW_INSTALL_SHA256}  /tmp/gh-aw/install-gh-aw.sh" | sha256sum -c -
  bash /tmp/gh-aw/install-gh-aw.sh
`

@copilot please address this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in efc3075: generateCopilotSetupStepsYAML now calls workflow.ResolveGhAwRef(ctx, "main") at call time to resolve the branch to an immutable commit SHA before building the raw URL. The three call-sites no longer share identical static content — each dynamically embeds the resolved SHA — so the drift concern is mitigated at the source.

Comment thread pkg/cli/copilot_setup.go Outdated
fmt.Fprintln(os.Stderr, " curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh | bash")
fmt.Fprintln(os.Stderr, " mkdir -p /tmp/gh-aw")
fmt.Fprintln(os.Stderr, " curl -fsSL https://raw.githubusercontent.com/github/gh-aw/refs/heads/main/install-gh-aw.sh -o /tmp/gh-aw/install-gh-aw.sh")
fmt.Fprintln(os.Stderr, " bash /tmp/gh-aw/install-gh-aw.sh")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[/codebase-design] The install script snippet is inlined as three separate fmt.Fprintln calls in the instructions renderer (lines 287–290) while the same snippet is a raw string literal in the workflow template constants above. These three copies will drift.

💡 Suggestion

Extract the install snippet to a single const or var used by both the template strings and the instructions renderer, so any future change (e.g., adding SHA256 verification) only needs to be made in one place.

@copilot please address this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in efc3075: both generateCopilotSetupStepsYAML (function return) and renderCopilotSetupUpdateInstructions now delegate to a single workflow.ResolveGhAwRef(ctx, "main") call site that resolves the SHA at runtime. The three formerly-identical inline snippets now share the same dynamic ref-resolution logic, eliminating the independent-drift risk.

@@ -0,0 +1,65 @@
#!/bin/bash

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot run script as part of makefile sync

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — added a sync-install-script-hashes target that calls scripts/update-install-script-hashes.sh, and wired it into the update target (runs after sync-action-pins, before the final rebuild). Help text updated accordingly.

…pdate

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🤖 PR Triage — §28848998303

Field Value
Category bug
Risk high
Score 75/100 (impact:38 urgency:25 quality:12)
Action 🚀 fast_track
Batch pr-batch:go-security

Carry-over × 4 — Security: RGS-018 curl|sh → download+verify. Bot change requests unresolved. Review together with #43766.

Generated by 🔧 PR Triage Agent · 84.1 AIC · ⌖ 6.65 AIC · ⊞ 5.4K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, confirm the branch is up to date with the base branch, and complete any remaining PR follow-up before rerunning checks.

Generated by 👨‍🍳 PR Sous Chef · 10.8 AIC · ⌖ 4.38 AIC · ⊞ 7.1K ·
Comment /souschef to run again

Copilot AI and others added 2 commits July 7, 2026 08:27
…is-report-2026-07-06

# Conflicts:
#	.github/workflows/daily-byok-ollama-test.lock.yml
#	.github/workflows/smoke-codex.lock.yml

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
…27.26)

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, confirm the branch is up to date with the base branch, and complete any remaining PR follow-up before rerunning checks.

...

Branch is now up to date with main (merged firewall bump 0.27.24 → 0.27.26, recompiled all lock files). Local validation passes (make fmt, make test-unit, make agent-report-progress). CI re-trigger needed from a maintainer (close/reopen or workflow_dispatch) to get green CI checks.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, make sure the branch is up to date with the base branch, and rerun checks.

Generated by 👨‍🍳 PR Sous Chef · 5.04 AIC · ⌖ 6.87 AIC · ⊞ 4.7K ·
Comment /souschef to run again

Copilot AI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, make sure the branch is up to date with the base branch, and rerun checks.

...

Branch is up to date with main (662e451) — 0 commits behind.

Local validation: make fmt, make test-unit, make agent-report-progress all pass (258 workflows compiled, zero lint errors, all lock files in sync).

Review threads: All 18 threads have been addressed. The two non-outdated unresolved threads (both by @pelikhan) already have copilot replies confirming the work is done — the PRRT_kwDOPc1QR86O0FFS thread ("run script as part of makefile sync") was addressed by wiring sync-install-script-hashes into the update Makefile target.

CI checks: All show action_required — standard for bot-pushed PRs that require maintainer approval before workflows execute. A maintainer close/reopen or workflow_dispatch will trigger CI.

  • ✅ Reviews — All threads addressed and replied to; no code changes needed this pass
  • ✅ Checks (local) — make fmt, make test-unit, make agent-report-progress green
  • ❓ Checks (CI) — Stale after agent push; needs human re-trigger. Prior agent runs all succeeded (success conclusion)
  • ✅ Mergeable — Branch up to date with main, no conflicts

Hand-off: A maintainer needs to approve CI execution (close/reopen or workflow_dispatch) before merge.

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🤖 PR Triage · Run §28868091227

Field Value
Category bug
Risk High
Score 75 / 100
Action fast_track
Batch pr-batch:go-security

Score breakdown: Impact 45 · Urgency 20 · Quality 10

Rationale: Security fix — replaces curl|sh installer pipes with download+verify (RGS-018 runner-guard rule). Carry-over x5. CI passing. @pelikhan + bot engaged. Bot CHANGES_REQUESTED cycles visible but dismissed. Needs expedited human review — security regression risk if not merged.

Labels: pr-type:bug pr-risk:high pr-priority:high pr-action:fast_track pr-batch:go-security

Generated by 🔧 PR Triage Agent · 119.4 AIC · ⌖ 6.65 AIC · ⊞ 5.4K ·

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill, address unresolved review comments, and rerun checks once the branch is up to date.

Generated by 👨‍🍳 PR Sous Chef · 3.94 AIC · ⌖ 7.46 AIC · ⊞ 7.1K ·
Comment /souschef to run again

…is-report-2026-07-06

# Conflicts:
#	.github/workflows/daily-byok-ollama-test.lock.yml

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill, address unresolved review comments, and rerun checks once the branch is up to date.

...

Branch merged with main (was 13 commits behind — ae04e75). Resolved conflict in daily-byok-ollama-test.lock.yml by recompiling from the MD source, so the lock file now includes both the Download activation artifact step from main and our OLLAMA env vars/SHA256 verification.

Local validation: make fmt, make test-unit, make agent-report-progress all pass (258 workflows compiled, zero lint errors, all lock files in sync).

  • ✅ Reviews — All threads addressed; no new review threads since last pass
  • ✅ Checks (local) — All pass
  • ❓ Checks (CI) — Stale after agent push; needs human re-trigger (close/reopen or workflow_dispatch)
  • ✅ Mergeable — Up to date with main, no conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[static-analysis] Report - 2026-07-06

4 participants