Skip to content

ci(renovate): auto-update SHA256 checksums for tirith and cosign - #5851

Merged
waynesun09 merged 1 commit into
mainfrom
hemartin/renovate-tirith-checksums
Aug 7, 2026
Merged

ci(renovate): auto-update SHA256 checksums for tirith and cosign#5851
waynesun09 merged 1 commit into
mainfrom
hemartin/renovate-tirith-checksums

Conversation

@rh-hemartin

@rh-hemartin rh-hemartin commented Aug 3, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds scripts/renovate/update-tirith-checksums.sh that fetches checksums.txt from the tirith GitHub release, verifies its cosign signature against the Sigstore transparency log, and patches TIRITH_SHA256_AMD64/ARM64 in the sandbox Containerfile. Cosign is not installed on the Renovate base image, so the script bootstraps it from a pinned version and SHA256.
  • Adds scripts/renovate/update-cosign-checksum.sh that downloads the new cosign binary, verifies its sigstore bundle using the old pinned binary, and updates COSIGN_SHA256 in update-tirith-checksums.sh.
  • Adds postUpgradeTasks to the tirith and cosign package rules in renovate.json so both scripts run after Renovate bumps their respective versions.
  • Adds a customManager so Renovate tracks the cosign version pin in update-tirith-checksums.sh.
  • Allows the scripts via RENOVATE_ALLOWED_POST_UPGRADE_COMMANDS in .github/workflows/renovate.yml.
  • Retains automerge: false for tirith so a human still reviews each version bump before merge.

Fixes: #5618

Test plan

  • Ran the tirith script locally against v0.3.1 (current); checksums unchanged
  • Simulated a tirith bump to v0.3.3; checksums updated correctly to match the release
  • Trigger a Renovate dry-run to verify postUpgradeTasks wiring (https://github.com/fullsend-ai/fullsend/actions/runs/30794383857/job/91624555395)
  • Verified cosign verify-blob passes against live v0.3.1 release artifacts with identity pinned to sheeki03/tirith and GitHub Actions OIDC issuer
  • Verify cosign checksum update script with a simulated cosign version bump

🤖 Generated with Claude Code

@rh-hemartin
rh-hemartin requested a review from a team as a code owner August 3, 2026 07:37
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

ci(renovate): auto-update tirith SHA256 checksums after version bumps

✨ Enhancement ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Run a post-upgrade script after Renovate bumps tirith to refresh pinned SHA256s.
• Allow Renovate workflow to execute the checksum refresh script safely.
• Remove manual-intervention automerge block for tirith patch updates.
Diagram

graph TD
  A["Renovate GitHub Action"] --> B["Renovate runner"] --> C["postUpgradeTasks"] --> D["update-tirith-checksums.sh"] --> E{{"GitHub Release checksums.txt"}} --> F["images/sandbox/Containerfile"]
  subgraph Legend
    direction LR
    _ci(["CI/Job"]) ~~~ _task(["Task/Script"]) ~~~ _ext{{"External"}} ~~~ _file["File"]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Renovate regex updates for SHA256 lines (no script)
  • ➕ No CI permission needed to run arbitrary commands
  • ➕ All updates remain within Renovate’s declarative config
  • ➖ Hard to express “fetch checksums.txt for version X then map to two ARG lines” purely via regex
  • ➖ More brittle/opaque than a short script; harder to troubleshoot when upstream naming changes
2. Build-time verification (download checksums.txt and validate) instead of pinning
  • ➕ No need to update pinned SHA256 values on version bumps
  • ➕ Keeps Containerfile simpler
  • ➖ Requires network access during image build and adds build-time flakiness
  • ➖ Weaker reproducibility/supply-chain posture than pinning exact expected checksums
3. Move tirith pin + checksums into a dedicated metadata file
  • ➕ Centralizes version/checksum data; easier for tooling and reviewers
  • ➕ Script can update a single small file rather than editing a Containerfile
  • ➖ Requires follow-up refactor and updating the build to read the metadata file
  • ➖ Doesn’t eliminate the need to fetch/derive checksums

Recommendation: The current postUpgradeTasks approach is the most practical balance of safety and maintainability: it keeps checksums pinned (reproducible builds) while removing the manual step that was breaking builds after Renovate bumps. The main thing to watch is robustness to upstream artifact naming changes; otherwise, this is preferable to trying to encode the mapping logic in Renovate regex alone.

Files changed (4) +34 / -5

Enhancement (2) +31 / -3
renovate.jsonRun checksum refresh postUpgradeTasks for tirith and re-enable patch automerge +7/-3

Run checksum refresh postUpgradeTasks for tirith and re-enable patch automerge

• Replaces the prior 'automerge: false' workaround with a postUpgradeTasks hook that runs the checksum update script after tirith bumps. Updates descriptions to reflect the now-automated checksum maintenance.

renovate.json

update-tirith-checksums.shAdd script to fetch tirith release checksums and patch Containerfile ARGs +24/-0

Add script to fetch tirith release checksums and patch Containerfile ARGs

• Introduces a bash script that reads the bumped TIRITH_VERSION from the sandbox Containerfile, downloads checksums.txt from the corresponding GitHub release, extracts amd64/arm64 sums, and rewrites the two SHA256 ARG lines. Fails fast if checksums cannot be extracted.

scripts/renovate/update-tirith-checksums.sh

Documentation (1) +2 / -2
ContainerfileDocument automated refresh for tirith pinned checksums +2/-2

Document automated refresh for tirith pinned checksums

• Updates comments above the tirith ARG pins to reflect that Renovate will bump the version and a post-upgrade script will refresh the SHA256 values. No functional build changes beyond documentation.

images/sandbox/Containerfile

Other (1) +1 / -0
renovate.ymlAllow Renovate to run post-upgrade scripts under scripts/renovate/ +1/-0

Allow Renovate to run post-upgrade scripts under scripts/renovate/

• Adds RENOVATE_ALLOWED_POST_UPGRADE_COMMANDS so Renovate can execute the new checksum refresh script. Restricts allowed commands to bash invocations under scripts/renovate/ for safety.

.github/workflows/renovate.yml

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:39 AM UTC · Completed 7:55 AM UTC
Commit: 7099360 · View workflow run →

@qodo-code-review

qodo-code-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Checksum extraction exits early ✓ Resolved 🐞 Bug ☼ Reliability
Description
In update-tirith-checksums.sh, set -euo pipefail combined with grep | awk pipelines in command
substitutions causes the script to exit immediately when a grep has no matches, so the later `[[
-z ... ]]` diagnostic is never reached. This makes Renovate postUpgradeTasks failures much harder to
debug when the upstream checksums format or artifact names change.
Code

scripts/renovate/update-tirith-checksums.sh[R13-16]

+AMD64=$(echo "$CHECKSUMS" | grep 'x86_64-unknown-linux-gnu\.tar\.gz' | awk '{print $1}')
+ARM64=$(echo "$CHECKSUMS" | grep 'aarch64-unknown-linux-gnu\.tar\.gz' | awk '{print $1}')
+
+if [[ -z "$AMD64" || -z "$ARM64" ]]; then
Relevance

●●● Strong

Team previously accepted fixing set -euo pipefail + grep no-match early-exit via || true (PR #3610;
similar in #390).

PR-#3610
PR-#390

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The script enables set -euo pipefail and then assigns AMD64/ARM64 via pipelines; with
pipefail, a no-match grep exits 1 and the assignment triggers errexit before the subsequent
-z check can run. This is the same failure mode previously accepted as a bug pattern in a similar
set -e + grep pipeline scenario.

scripts/renovate/update-tirith-checksums.sh[5-19]
PR-#3610

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`set -euo pipefail` makes the checksum extraction assignments fail-fast when `grep` returns exit code 1 (no matches). This exits the script before the explicit `if [[ -z ... ]]` block can emit the intended error message.

## Issue Context
This script is run as a Renovate `postUpgradeTasks` command. When extraction fails, we want a clear, intentional diagnostic (`could not extract checksums...`) rather than an abrupt exit during assignment.

## Fix Focus Areas
- scripts/renovate/update-tirith-checksums.sh[5-19]

### Suggested approach
- Ensure the extraction command substitutions *do not* cause `errexit` on no-match, then rely on the existing `-z` check. For example:
 - `AMD64=$(echo "$CHECKSUMS" | grep -m1 ... | awk '{print $1}' || true)` (same for ARM64), **or**
 - replace the pipeline with an `awk` that exits 0 even when no match is found (then `-z` check handles it), **or**
 - temporarily disable `set -e` around the extraction step and restore it immediately after.
- Keep `pipefail` enabled for the rest of the script.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Overbroad Renovate allowlist ✗ Dismissed 🐞 Bug ⛨ Security
Description
The workflow allowlists post-upgrade commands with the prefix regex ^bash scripts/renovate/, which
permits any command starting with that prefix, not just the single intended checksum-update script.
Tightening this to an exact, anchored command reduces the chance that a future Renovate config
change unintentionally expands what can execute in CI.
Code

.github/workflows/renovate.yml[50]

+          RENOVATE_ALLOWED_POST_UPGRADE_COMMANDS: '["^bash scripts/renovate/"]'
Relevance

●● Moderate

No direct history on Renovate command allowlist; team often accepts regex tightening in workflows
(PRs #2398, #225).

PR-#2398
PR-#225

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow sets a prefix-based allowlist, while the Renovate configuration only invokes one
specific script; the allowlist can be tightened without impacting current functionality.

.github/workflows/renovate.yml[42-50]
renovate.json[32-39]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RENOVATE_ALLOWED_POST_UPGRADE_COMMANDS` is configured as a broad prefix regex, which weakens the intended safety boundary of limiting which post-upgrade commands Renovate may execute.

## Issue Context
The repo currently only needs to allow one command: `bash scripts/renovate/update-tirith-checksums.sh`.

## Fix Focus Areas
- .github/workflows/renovate.yml[47-50]
- renovate.json[32-39]

### Suggested approach
- Change the allowlist entry to a fully anchored regex matching only the intended command, e.g.:
 - `^bash scripts/renovate/update-tirith-checksums\.sh$`
- If you intentionally want to allow multiple scripts in the directory, document that intent and consider still anchoring to `\.sh$` (and optionally disallowing extra args) to keep the boundary crisp.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
✅ Compliance rules (platform): 54 rules

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread scripts/renovate/update-tirith-checksums.sh Outdated
Comment thread .github/workflows/renovate.yml Outdated
@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] .github/workflows/renovate.yml, images/sandbox/Containerfile, scripts/renovate/update-cosign-checksum.sh, scripts/renovate/update-tirith-checksums.sh — This PR modifies files under protected paths (.github/, images/, scripts/). The PR links to issue chore(deps): update dependency sheeki03/tirith to v0.3.3 #5618 and explains the rationale for these changes. Human approval is always required for protected-path changes, regardless of context.

Low

  • [indentation-consistency] scripts/renovate/update-tirith-checksums.sh:12 — This script uses 4-space indentation for if/else blocks, while update-cosign-checksum.sh (in the same directory, same PR) and existing scripts in scripts/ (check-e2e-authorization.sh) all use 2-space indentation. The two new scripts in the same PR should at least be consistent with each other.

  • [naming-convention] scripts/renovate/update-cosign-checksum.sh — Asymmetric naming: update-cosign-checksum.sh (singular) vs update-tirith-checksums.sh (plural). Both scripts perform the same role (refreshing checksums after a version bump). The singular form is defensible since cosign updates one checksum while tirith updates two, but a consistent plural form would be more uniform.

Previous run

Review

Findings

Medium

  • [operational-logic] renovate.json:33 — The cosign packageRule has postUpgradeTasks to auto-update COSIGN_SHA256 but does not override automerge. The top-level rule automerges patch bumps. If the postUpgradeTask fails (e.g., network error or signature-verification failure), Renovate still creates the PR with the version bumped and the old COSIGN_SHA256. The automerge rule then merges the stale checksum. The next tirith version bump's postUpgradeTask then fails at sha256sum verification when bootstrapping cosign, breaking the tirith automation until the checksum is corrected manually.
    Remediation: Add "automerge": false to the sigstore/cosign packageRule, matching the pattern used for sheeki03/tirith.

  • [protected-path] .github/workflows/renovate.yml, images/sandbox/Containerfile, scripts/renovate/update-cosign-checksum.sh, scripts/renovate/update-tirith-checksums.sh — This PR modifies files under protected paths (.github/, images/, scripts/). The PR links to issue chore(deps): update dependency sheeki03/tirith to v0.3.3 #5618 and explains the rationale for these changes. Human approval is always required for protected-path changes, regardless of context.

Low

  • [supply-chain] scripts/renovate/update-tirith-checksums.sh:10VERSION is extracted via grep -oP 'ARG TIRITH_VERSION=\K\S+', which matches any non-whitespace. The Renovate customManager constrains versions to \d+\.\d+\.\d+ and cosign verification provides defense-in-depth, but the script does not independently validate the version format before interpolating into URLs and sed patterns.
    Remediation: Add [[ ! "$VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]] && { echo "error: invalid version" >&2; exit 1; } after extraction.

  • [supply-chain] scripts/renovate/update-cosign-checksum.sh:11OLD_VERSION and NEW_VERSION are extracted via grep -oP matching any non-whitespace and interpolated into curl URLs without format validation. Same defense-in-depth reasoning applies (Renovate regex constrains input), but independent validation provides an extra layer.
    Remediation: Validate both match ^[0-9]+\.[0-9]+\.[0-9]+$ before use.

  • [naming-convention] scripts/renovate/update-cosign-checksum.sh — Variable references throughout both new scripts use bare $VAR style (e.g., $SCRIPT, $OLD_BINARY, $WORKDIR), while existing scripts in this repo (check-e2e-authorization.sh) consistently use brace-quoted ${VAR} form (~20 references across both files).

  • [naming-convention] scripts/renovate/update-cosign-checksum.sh:22 — Unquoted command substitution in WORKDIR=$(mktemp -d) (also in update-tirith-checksums.sh). Existing convention quotes command substitutions: TMPDIR="$(mktemp -d)".


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (2)

Review

Findings

High

  • [runtime-mechanism] renovate.json — Both the tirith and cosign postUpgradeTasks use executionMode: "update" and modify the same file that the custom regex manager already changed (tirith: images/sandbox/Containerfile, cosign: scripts/renovate/update-tirith-checksums.sh). This triggers a confirmed Renovate bug (renovatebot/renovate#42263) where executionMode: "update" silently drops file changes made by postUpgradeTasks when they modify the same file as the manager update. The SHA256 checksum updates written by the scripts will be silently discarded from the commit, resulting in version bumps without corresponding checksum updates — exactly the failure mode this PR intends to eliminate. The dry-run test would not catch this because dry-runs do not create actual commits.
    Remediation: Change executionMode from "update" to "branch" in both postUpgradeTasks blocks. "branch" runs the command once per branch after all updates are committed, and file changes are committed as a separate step, avoiding the same-file collision.

Medium

  • [operational-logic] renovate.json — The cosign packageRule has postUpgradeTasks to auto-update COSIGN_SHA256, but no automerge: false override. The top-level rule automerges patch bumps. Combined with the executionMode: "update" bug above, a cosign patch bump could be automerged with a stale COSIGN_SHA256, causing the tirith update script to fail on the next tirith version bump (sha256sum -c rejects the mismatch). Adding automerge: false provides defense-in-depth.
    Remediation: Add "automerge": false to the cosign packageRule, matching the pattern used for the tirith rule.

  • [protected-path] .github/workflows/renovate.yml, images/sandbox/Containerfile, scripts/renovate/update-cosign-checksum.sh, scripts/renovate/update-tirith-checksums.sh — This PR modifies files under protected paths (.github/, images/, scripts/). The PR links to issue chore(deps): update dependency sheeki03/tirith to v0.3.3 #5618 and explains the rationale for these changes. Human approval is always required for protected-path changes, regardless of context.

Low

  • [command-injection] .github/workflows/renovate.yml:50RENOVATE_ALLOWED_COMMANDS regex ^bash scripts/renovate/ lacks an end anchor ($). Without shell execution (allowShellExecutorForPostUpgradeCommands not enabled), the practical risk is low. Adding an end anchor provides defense-in-depth.
    Remediation: Tighten to ^bash scripts/renovate/[a-zA-Z0-9_-]+\.sh$.

  • [typo] scripts/renovate/update-tirith-checksums.sh:8 — Comment says "Tirtih" instead of "Tirith" on line 8 (# --- Retrieve the new Tirtih version ---). Same typo on line 12 in the error message (Tried to retrieve Tirtih version). The error message is user-facing in Renovate logs.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (3)

Review

Findings

Medium

  • [operational-logic] renovate.json:107 — The new cosign customManager tracks COSIGN_VERSION in the script, but there is no packageRule disabling automerge for sigstore/cosign. The default rule (line 8) automerges patch bumps. When Renovate bumps cosign's version, only COSIGN_VERSION is updated — COSIGN_SHA256 is not (the description says it must be refreshed manually). If a cosign patch bump is automerged, the next tirith version bump will fail during postUpgradeTasks because sha256sum -c rejects the stale hash. While the failure is safe (fail-closed), it creates non-obvious breakage: the cosign PR succeeds and merges, but a later tirith PR fails.
    Remediation: Add a packageRule { "matchPackageNames": ["sigstore/cosign"], "automerge": false } so cosign version bumps require manual SHA256 refresh before merge.

  • [protected-path] .github/workflows/renovate.yml, images/sandbox/Containerfile, scripts/renovate/update-tirith-checksums.sh — This PR modifies files under protected paths: .github/workflows/renovate.yml (.github/), images/sandbox/Containerfile (images/), and scripts/renovate/update-tirith-checksums.sh (scripts/). The PR links to issue chore(deps): update dependency sheeki03/tirith to v0.3.3 #5618 and explains the rationale for these changes. Human approval is always required for protected-path changes, regardless of context.

Low

  • [typo] scripts/renovate/update-tirith-checksums.sh:8 — Comment says "Tirtih" instead of "Tirith". Same typo on line 12 in the error message. The error message is user-facing in Renovate logs.

  • [script-header-format] scripts/renovate/update-tirith-checksums.sh:2 — Script header comment does not follow the established # <filename> — <description>. pattern used by other scripts in scripts/.


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR
Previous run (4)

Review

Findings

Medium

  • [variable-shadowing] scripts/renovate/update-tirith-checksums.sh:18TMPDIR is a standard POSIX environment variable used by mktemp and many other tools to locate the temporary-file directory. Reassigning it to the script's own scratch directory means child processes (including cosign verify-blob) would use the script's private temp directory instead of the system default. Rename to SCRATCH_DIR or WORKDIR to avoid shadowing the POSIX variable.

  • [protected-path] .github/workflows/renovate.yml, images/sandbox/Containerfile, scripts/renovate/update-tirith-checksums.sh — This PR modifies files under protected paths: .github/workflows/renovate.yml (.github/), images/sandbox/Containerfile (images/), and scripts/renovate/update-tirith-checksums.sh (scripts/). The PR links to issue chore(deps): update dependency sheeki03/tirith to v0.3.3 #5618 and explains the rationale for these changes. Human approval is always required for protected-path changes, regardless of context.

Low

  • [error-handling] scripts/renovate/update-tirith-checksums.sh:9 — If the Containerfile does not contain a line matching ARG TIRITH_VERSION=, grep -oP exits non-zero and set -e terminates the script with no explanatory message. Unlikely since Renovate only runs this after bumping TIRITH_VERSION.

  • [permission-expansion] .github/workflows/renovate.yml:50RENOVATE_ALLOWED_POST_UPGRADE_COMMANDS regex ^bash scripts/renovate/ anchors at the start but not the end, permitting any future script in that directory. Consider tightening to ^bash scripts/renovate/update-tirith-checksums\\.sh$.

  • [script-header-format] scripts/renovate/update-tirith-checksums.sh:2 — Script header comment does not follow the established # <filename> — <description>. pattern used by other scripts in scripts/.

  • [auditability-gap] scripts/renovate/update-tirith-checksums.sh — Script output (cosign verification result, extracted checksums) is only visible in Renovate job logs, not captured in git history. Renovate's log capture provides the audit trail.

  • [command-injection] scripts/renovate/update-tirith-checksums.sh:63$AMD64 and $ARM64 are interpolated into sed replacement patterns. The validation on lines 56–62 constrains values to ^[0-9a-f]{64}$, so the current code is safe. Noted for defense-in-depth.

  • [supply-chain] scripts/renovate/update-tirith-checksums.sh:16 — Cosign binary download is pinned to cosign-linux-amd64 only. If Renovate runs on arm64, the SHA256 check catches the mismatch (fail-closed), but the script would fail.

  • [naming-alignment] scripts/renovate/update-tirith-checksums.sh:10 — Containerfile path is hardcoded rather than parameterized, coupling it to the fileFilters declaration in renovate.json.

Previous run (5)

Review

Findings

Medium

  • [protected-path] .github/workflows/renovate.yml, images/sandbox/Containerfile, scripts/renovate/update-tirith-checksums.sh — This PR modifies files under protected paths: .github/workflows/renovate.yml (.github/), images/sandbox/Containerfile (images/), and scripts/renovate/update-tirith-checksums.sh (scripts/). The PR links to issue chore(deps): update dependency sheeki03/tirith to v0.3.3 #5618 and explains the rationale for these changes. Human approval is always required for protected-path changes, regardless of context.

Low

  • [error-handling] scripts/renovate/update-tirith-checksums.sh:8 — If the Containerfile does not contain a line matching ARG TIRITH_VERSION=, grep -oP exits non-zero and set -e terminates the script with no explanatory message. Unlikely since Renovate only runs this after bumping TIRITH_VERSION.

  • [directory-organization] scripts/renovate/update-tirith-checksums.sh — Creates new subdirectory scripts/renovate/ while existing scripts in scripts/ use a flat structure. Reasonable for namespacing Renovate-specific automation but diverges from the current pattern.

Previous run (6)

Review

Findings

Medium

  • [command-injection] scripts/renovate/update-tirith-checksums.sh:13 — The grep patterns for extracting checksums are not anchored and could match multiple lines in checksums.txt if the release includes related artifacts (e.g., .tar.gz.sha256, .tar.gz.sig). If grep matches more than one line, awk '{print $1}' produces a multi-line string, and the subsequent sed substitution would inject a malformed multi-line ARG value into the Containerfile. Additionally, the extracted values are interpolated into sed -i without validation — a compromised checksums.txt could contain sed metacharacters.
    Remediation: Add validation after extraction: [[ "$AMD64" =~ ^[0-9a-f]{64}$ ]] || { echo 'error: AMD64 checksum is not a valid sha256 hex digest' >&2; exit 1; } (and similarly for ARM64).

  • [permission-expansion] renovate.json:33 — This PR removes the automerge: false override for sheeki03/tirith. Combined with the top-level automerge: true rule for patch/pin updates, tirith patch bumps will now automerge without human review. The existing supply-chain safeguard (SHA256 verification at build time) becomes circular since the checksums are now auto-fetched from the same upstream source. A compromise of sheeki03/tirith could inject a malicious binary whose checksums would be automatically fetched, committed, and automerged.
    Remediation: Add "automerge": false to the tirith package rule alongside postUpgradeTasks, preserving human review while automating the checksum update.

  • [protected-path] .github/workflows/renovate.yml — This PR modifies files under protected paths: .github/workflows/renovate.yml, images/sandbox/Containerfile, scripts/renovate/update-tirith-checksums.sh. The PR links to issue chore(deps): update dependency sheeki03/tirith to v0.3.3 #5618 and explains the rationale. Human approval is always required for protected-path changes, regardless of context.

Low

  • [error-handling] scripts/renovate/update-tirith-checksums.sh:10 — The curl -fsSL call will abort on HTTP errors via set -e, but if GitHub returns 200 with non-checksums content (rate limiting), the grep/awk pipeline will extract empty strings. The validation on line 16 catches this, but the error message ("could not extract checksums") won't indicate the actual cause.

  • [directory-organization] scripts/renovate/update-tirith-checksums.sh — Creates new subdirectory scripts/renovate/ while existing scripts in scripts/ use a flat structure. The choice is reasonable for namespacing Renovate-specific automation but diverges from the current pattern.


Labels: PR modifies CI/Renovate configuration and sandbox image build


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added component/ci CI pipelines and checks component/sandbox OpenShell sandbox environment labels Aug 3, 2026
@rh-hemartin
rh-hemartin force-pushed the hemartin/renovate-tirith-checksums branch from 7099360 to 61bb450 Compare August 3, 2026 08:09
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:11 AM UTC · Completed 8:25 AM UTC
Commit: 61bb450 · View workflow run →

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review August 3, 2026 08:24

Superseded by updated review

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 3, 2026
@rh-hemartin rh-hemartin self-assigned this Aug 3, 2026

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review-only pass (no approval/changes requested). Two inline findings above; one additional finding that isn't tied to a specific line:

[MEDIUM] PR's own linked dry-run evidence shows postUpgradeTasks failing, not succeeding

The PR test plan cites a Renovate dry-run (run 30794383857, job 91624555395) as proof postUpgradeTasks wiring works, but the actual log from that run shows an artifactError with Command failed: bash scripts/renovate/update-tirith-checksums.sh ... No such file or directory, and Renovate falling back to a dry-run lock-file-error comment — i.e. the postUpgradeTasks command never actually executed successfully in the cited evidence. This happens because the dry run checked out main, which doesn't yet have the new script (only this PR's branch does) — a chicken-and-egg gap. So the actual post-merge behavior of the new automation (does it correctly patch the Containerfile end-to-end against a real upstream release) has never been demonstrated to work; the cited "proof" actually demonstrates the opposite.

Suggestion: Re-run the dry-run pointed at a branch/config where the script already exists (e.g. RENOVATE_BASE_BRANCHES set to this PR's branch) and link a run showing the script executing successfully end-to-end, or explicitly caveat in the PR description that the wiring is unverified until the first real run against main post-merge, with a plan to check that run.

Comment thread scripts/renovate/update-tirith-checksums.sh Outdated
Comment thread renovate.json
@rh-hemartin
rh-hemartin force-pushed the hemartin/renovate-tirith-checksums branch from 61bb450 to 904360d Compare August 3, 2026 13:41
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 3, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 1:42 PM UTC · Ended 2:00 PM UTC
Commit: 904360d · View workflow run →

@rh-hemartin

Copy link
Copy Markdown
Member Author

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:42 PM UTC · Completed 2:00 PM UTC
Commit: 904360d · View workflow run →

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review-only pass (no approval/changes requested). Two new inline findings below, not previously raised in the existing bot/human review threads on this PR.

Comment thread scripts/renovate/update-tirith-checksums.sh Outdated
Comment thread scripts/renovate/update-tirith-checksums.sh
@rh-hemartin
rh-hemartin force-pushed the hemartin/renovate-tirith-checksums branch from 904360d to dd41b03 Compare August 4, 2026 05:45
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 4, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 5:47 AM UTC · Ended 5:47 AM UTC
Commit: dd41b03 · View workflow run →

@rh-hemartin
rh-hemartin force-pushed the hemartin/renovate-tirith-checksums branch from dd41b03 to 81dfb61 Compare August 4, 2026 05:47
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 4, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:48 AM UTC · Completed 6:05 AM UTC
Commit: 81dfb61 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot removed the requires-manual-review Review requires human judgment label Aug 4, 2026

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review-only sweep (no assignment, no approve/request-changes). Both findings below are independently verified and distinct from the existing discussion threads on this PR.

Comment thread renovate.json Outdated
Comment thread .github/workflows/renovate.yml Outdated
@rh-hemartin rh-hemartin changed the title ci(renovate): auto-update tirith SHA256 checksums on version bump ci(renovate): auto-update SHA256 checksums for tirith and cosign Aug 5, 2026
@rh-hemartin
rh-hemartin force-pushed the hemartin/renovate-tirith-checksums branch from 81dfb61 to 96cdc3d Compare August 5, 2026 09:20
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 5, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:20 AM UTC · Completed 10:02 AM UTC
Commit: 96cdc3d · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Review-only pass (no approval/changes requested, no self-assignment). Two verified findings below.

Comment thread renovate.json
Comment thread scripts/renovate/update-tirith-checksums.sh
@rh-hemartin
rh-hemartin force-pushed the hemartin/renovate-tirith-checksums branch from 96cdc3d to 9b63ee3 Compare August 6, 2026 07:34
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 6, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 7:35 AM UTC · Ended 7:40 AM UTC
Commit: 9b63ee3 · View workflow run →

@rh-hemartin
rh-hemartin force-pushed the hemartin/renovate-tirith-checksums branch from 9b63ee3 to 2347bb6 Compare August 6, 2026 07:40
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 6, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:41 AM UTC · Completed 8:00 AM UTC
Commit: 2347bb6 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Comment thread scripts/renovate/update-cosign-checksum.sh Outdated
Renovate bumps TIRITH_VERSION but left TIRITH_SHA256_{AMD64,ARM64}
stale, breaking the sandbox image build. Add a postUpgradeTasks script
that fetches checksums.txt from the new release and patches the
Containerfile ARGs automatically.

This process needed cosign, which is not installed on Renovate base
image. Add a Renovate rule to update the Cosign binary installed
within `scripts/renovate/update-tirith-checksums.sh`. To update
cosign itself another script is added which first downloads the previous
cosign version and then uses it to verify the next cosign version.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Hector Martinez <hemartin@redhat.com>
@rh-hemartin
rh-hemartin force-pushed the hemartin/renovate-tirith-checksums branch from 2347bb6 to 318c987 Compare August 7, 2026 06:20
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 7, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 6:22 AM UTC · Completed 6:40 AM UTC
Commit: 318c987 · View workflow run →

@fullsend-ai-review
fullsend-ai-review Bot dismissed stale reviews from themself August 7, 2026 06:40

Superseded by updated review

@fullsend-ai-review fullsend-ai-review 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.

See the review comment for full details.

Comment thread scripts/renovate/update-tirith-checksums.sh
@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Aug 7, 2026

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(Duplicate submission from a retried CLI call — see the following approval review for the full validation write-up.)

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving. I validated the tirith version-lock and install chain end to end by actually executing both scripts, not just reading them.

What I ran

  1. update-tirith-checksums.sh in the real Renovate runtimeghcr.io/renovatebot/renovate:43 (the default image for the pinned renovatebot/github-action v46.1.21), linux/amd64, with the Containerfile pre-bumped to TIRITH_VERSION=0.3.3 and a deliberately wrong TIRITH_SHA256_AMD64. Result: cosign bootstrap sha256sum -c → OK, verify-blob on checksums.txtVerified OK, both ARGs rewritten, exit 0.
  2. Toolchain check on that same imagecurl, git, sha256sum, mktemp, sed -i, GNU grep 3.11 (so grep -oP works), and outbound HTTPS to GitHub releases all present/working. Worth confirming explicitly since the whole design rests on the Renovate image being able to fetch and hash.
  3. update-cosign-checksum.sh against a real pending bump — cosign v3.1.3 shipped 2026-08-06, so I simulated exactly the bump Renovate will produce (3.1.2 → 3.1.3) in a throwaway git repo: old binary bootstrapped and checksum-verified, new binary's sigstore bundle verified with the old binary (Verified OK), COSIGN_SHA256 rewritten. The keyless@projectsigstore.iam.gserviceaccount.com / accounts.google.com identity pin is correct for current cosign releases.

Cross-checks against upstream (independent of the scripts)

  • Written amd64/arm64 SHAs match tirith v0.3.3's signed checksums.txt exactly (6cdbe35e…, c784233…).
  • Downloading tirith-x86_64-unknown-linux-gnu.tar.gz v0.3.3 hashes to the amd64 value the script wrote, and the tarball has tirith as a top-level member — so the Containerfile's sha256sum -c - + tar xzf -C /usr/local/bin tirith install step succeeds with what the script produces. The amd64→x86_64 / arm64→aarch64 mapping matches the release asset names.
  • Computed cosign SHA 4629c757… matches sigstore's own cosign_checksums.txt for v3.1.3.
  • checksums.txt uses the two-space sha256␣␣filename format the anchored greps require, and only one line matches each pattern.
  • RENOVATE_ALLOWED_COMMANDS is the correct (non-deprecated) name for Renovate 43, and the anchored ^bash scripts/renovate/[a-zA-Z0-9_-]+\.sh$ regex admits both scripts.

Failure modes are all fail-closed: a bad cosign bootstrap hash, a missing/invalid signature, a non-semver version, or a malformed digest each abort before any sed, leaving the pinned checksums untouched and the bump PR blocked. automerge: false on both rules keeps a human in the loop either way.

Non-blocking notes (nothing here needs a change in this PR)

  • cosign 3.x prints deprecation warnings for --certificate/--signature (please use --bundle with --trusted-root). Works today; a future cosign major that removes them would break tirith bumps — fail-closed, but worth a follow-up when it lands.
  • The command -v cosign fallback trusting a PATH cosign is a deliberate, already-discussed trade-off; fine as a follow-up if the base image ever ships one.
  • The remaining open thread is the 4-space vs 2-space indentation nit — cosmetic.

@waynesun09
waynesun09 enabled auto-merge August 7, 2026 12:58
@waynesun09
waynesun09 added this pull request to the merge queue Aug 7, 2026
Merged via the queue into main with commit e7db4f1 Aug 7, 2026
25 checks passed
@waynesun09
waynesun09 deleted the hemartin/renovate-tirith-checksums branch August 7, 2026 13:06
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 7, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 1:08 PM UTC · Completed 1:24 PM UTC

Commit: 318c987 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5851 — ci(renovate): auto-update SHA256 checksums for tirith and cosign

Workflow Overview

Human-authored PR by rh-hemartin (co-authored with Claude Code) adding Renovate post-upgrade scripts to auto-update SHA256 checksums for tirith and cosign. The PR went through 7 review agent iterations over 4 days (Aug 3–7), with extensive human review from waynesun09. Merged after thorough validation.

Agents repo: fullsend-ai/agents (discovered from review dispatch run 31153637658).

Review Quality Delta

The human reviewer (waynesun09) consistently found higher-impact issues than the review agent. Key findings by source:

Human found, agent never found (7 issues):

  • [HIGH] Unsigned checksums.txt fetched without cosign signature verification — the fundamental supply-chain gap
  • [MEDIUM] Deprecated RENOVATE_ALLOWED_POST_UPGRADE_COMMANDS env var (renamed to RENOVATE_ALLOWED_COMMANDS)
  • [MEDIUM] certificate-identity-regexp broader than upstream install.sh
  • [MEDIUM] Hardcoded cosign version/checksum with no Renovate tracking (the same manual-maintenance problem the PR was solving)
  • [MEDIUM] Cosign bootstrap silently trusts pre-existing cosign on PATH
  • [MEDIUM] Sibling script update-cosign-checksum.sh lacks fail-with-message guards present in update-tirith-checksums.sh
  • [CRITICAL] Live verification via API that checksum-refresh was broken on dependent PR chore(deps): update dependency sheeki03/tirith to v0.3.3 #5618

Human found first, agent found 1 day later (1 issue):

  • [CRITICAL] executionMode: "update" collision with Renovate bug #42263 — human found Aug 5, agent found Aug 6

Agent found first (3 issues):

  • Command injection risk (unanchored grep + sed interpolation)
  • Variable shadowing (TMPDIR is a POSIX env var)
  • Style/naming issues

What made the human review superior: domain-specific Renovate expertise (deprecated env vars, executionMode bugs), live state verification (checking PR #5618 via API, running scripts in the Renovate container), and systemic thinking (detecting the irony of hardcoding cosign while automating tirith).

What Went Well

  • The review agent's challenger correctly dismissed a false-positive scope-creep finding on the final review
  • The requires-manual-review label appropriately constrained the agent to comment-only disposition
  • The agent's correctness sub-agent on the final run used web search to verify Renovate env var naming
  • The agent found genuine command-injection risks the human didn't raise

Evidence Supporting Existing Issues

Proposals filed

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Automated review sweep: 2 additional findings below.

"${COSIGN}" verify-blob \
--certificate "${WORKDIR}/checksums.txt.pem" \
--signature "${WORKDIR}/checksums.txt.sig" \
--certificate-identity-regexp "^https://github\\.com/sheeki03/tirith/\\.github/workflows/" \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[MEDIUM] cosign certificate-identity-regexp for tirith accepts any .github/workflows/ file, not just release.yml

Independently verified by decoding the real checksums.txt.pem for tirith v0.3.1: the certificate SAN is exactly https://github.com/sheeki03/tirith/.github/workflows/release.yml@refs/tags/v0.3.1. The --certificate-identity-regexp here is anchored only at the start (^https://github\.com/sheeki03/tirith/\.github/workflows/), so it would accept a Fulcio cert minted by ANY workflow under that path, not just release.yml — confirmed via the GitHub API that sheeki03/tirith also has ci.yml, bench.yml, fuzz.yml, and threatdb.yml. None of those currently declare id-token: write (also confirmed), so this isn't exploitable today, but the trust boundary is looser than it needs to be: if any workflow in that upstream, third-party repo is ever granted OIDC signing permission for any reason, it could mint a signature this script would accept as a valid tirith "release". This is distinct from the earlier resolved thread on this line (which made the regex match upstream install.sh's .github/workflows/ prefix) — that fix matched upstream's scope, but upstream's scope itself doesn't pin to the specific release workflow.

Suggestion: anchor the regex to the exact release workflow and version being fetched, e.g. --certificate-identity-regexp "^https://github\\.com/sheeki03/tirith/\\.github/workflows/release\\.yml@refs/tags/v${VERSION}$", so a compromised or newly-added non-release workflow with id-token: write can't forge a valid signature.

SCRIPT="scripts/renovate/update-tirith-checksums.sh"

# --- Read old and new versions ---
OLD_VERSION=$(git show HEAD:"${SCRIPT}" | grep -oP '^COSIGN_VERSION=\K\S+' || true)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[MEDIUM] OLD_VERSION extraction via git show HEAD assumes an unconfirmed Renovate commit ordering for executionMode: branch

OLD_VERSION/OLD_SHA256 are read via git show HEAD:"${SCRIPT}" while NEW_VERSION is read from the working tree. This only produces a correct diff if Renovate applies the COSIGN_VERSION bump to the working tree but has NOT yet committed it to HEAD by the time postUpgradeTasks runs. The cosign packageRule in renovate.json now uses executionMode: "branch" (changed in this same PR specifically to work around the previously-flagged 'update'-mode same-file collision, renovatebot/renovate#42263). I could not confirm from Renovate's public docs whether 'branch' mode commits the manager's version bump before or after invoking postUpgradeTasks. If the bump is committed first, git show HEAD would already return the NEW version, OLD_VERSION would equal NEW_VERSION, and the script would print "cosign version unchanged (...)" and exit 0 without verifying the new binary or refreshing COSIGN_SHA256 — a silent no-op that looks successful in CI logs.

Suggestion: confirm Renovate's actual commit ordering for executionMode: branch against a real cosign version bump before relying on this, or make the script resilient to either ordering (e.g. derive OLD_VERSION from the parent commit git show HEAD~1:... or add an explicit log/guard that treats an unexpected 'unchanged' result as suspicious when the Renovate PR title indicates a bump).

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

Labels

component/ci CI pipelines and checks component/sandbox OpenShell sandbox environment requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants