Skip to content

chore: restore simplified on-child-update workflow #62

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mateodelnorte merged 1 commit into from
Mar 26, 2026
Merged

chore: restore simplified on-child-update workflow #62

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions .github/workflows/on-child-update.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
name: On Child Repo Update

on:
repository_dispatch:
types: [child-repo-updated]

concurrency:
group: child-repo-sync
cancel-in-progress: false

permissions:
contents: write
pull-requests: write

jobs:
create-sync-pr:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4
with:
token: ${{ secrets.PARENT_REPO_PAT }}

- name: Create sync PR
env:
REPO_NAME: ${{ github.event.client_payload.repo_name }}
FULL_SHA: ${{ github.event.client_payload.sha }}
GH_TOKEN: ${{ secrets.PARENT_REPO_PAT }}
run: |
git config user.name "github-actions[bot]"
git config user.email "41898282+github-actions[bot]@users.noreply.github.com"

REPO_NAME="${REPO_NAME:-unknown}"
SHORT_SHA="${FULL_SHA:0:7}"
SHORT_SHA="${SHORT_SHA:-$(date +%s | tail -c 8)}"

BRANCH="sync/${REPO_NAME}/${SHORT_SHA}"
PR_TITLE="chore(${REPO_NAME}): sync ${SHORT_SHA}"

Comment on lines +33 to +39

@coderabbitai coderabbitai Bot Mar 26, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/on-child-update.yml

Repository: harmony-labs/meta

Length of output: 2893

🏁 Script executed:

find . -type f \( -name "*.md" -o -name "*.txt" -o -name "*test*" -o -name "*spec*" \) | head -20

Repository: harmony-labs/meta

Length of output: 762

🏁 Script executed:

rg -i "client_payload|repository_dispatch|REPO_NAME|SHORT_SHA" --type md --type txt

Repository: harmony-labs/meta

Length of output: 220

🏁 Script executed:

rg "repository_dispatch|on:.*child-repo-updated" --type yaml -A 5 -B 5

Repository: harmony-labs/meta

Length of output: 43

🏁 Script executed:

find . -name "*.md" -exec rg -l "child.*repo|payload.*struct|dispatch" {} \;

Repository: harmony-labs/meta

Length of output: 510

🏁 Script executed:

grep -n "repo_name\|short_sha\|actor\|payload" README.md | head -20

Repository: harmony-labs/meta

Length of output: 324

🏁 Script executed:

grep -n "child.*repo\|dispatch" ./docs/*.md

Repository: harmony-labs/meta

Length of output: 1632

🏁 Script executed:

rg "repository_dispatch\|gh.*dispatch\|client_payload" --type yml --type yaml --type sh --type bash -B 3 -A 3

Repository: harmony-labs/meta

Length of output: 88

🏁 Script executed:

find . -type f -name "*.yml" -o -name "*.yaml" | xargs grep -l "dispatch" 2>/dev/null

Repository: harmony-labs/meta

Length of output: 169

🏁 Script executed:

cat -n .github/workflows/on-push-main.yml

Repository: harmony-labs/meta

Length of output: 1728

🏁 Script executed:

cat -n .github/workflows/release.yml

Repository: harmony-labs/meta

Length of output: 15447

🏁 Script executed:

rg "short_sha|SHORT_SHA" -i --type md

Repository: harmony-labs/meta

Length of output: 43

Validate and sanitize dispatch payload fields before using them in git refs and commit metadata.

REPO_NAME, SHORT_SHA, and ACTOR are sourced from github.event.client_payload and used directly in branch names, PR titles, and commit messages. Malformed payload values can cause git operations to fail, and the SHORT_SHA default ($(date +%s | tail -c 8)) produces an 8-digit timestamp instead of a commit hash, undermining traceability. Additionally, unvalidated values in the Co-authored-by trailer can produce malformed email addresses.

Proposed hardening patch 
-          REPO_NAME="${REPO_NAME:-unknown}"
-          SHORT_SHA="${SHORT_SHA:-$(date +%s | tail -c 8)}"
-          ACTOR="${ACTOR:-github-actions}"
+          : "${REPO_NAME:?Missing client_payload.repo_name}"
+          : "${SHORT_SHA:?Missing client_payload.short_sha}"
+          ACTOR="${ACTOR:-github-actions}"
+
+          SAFE_REPO_NAME="$(printf '%s' "$REPO_NAME" | tr '[:upper:]' '[:lower:]' | sed -E 's#[^a-z0-9._/-]+#-#g; s#(^/+|/+$)##g')"
+          SAFE_SHORT_SHA="$(printf '%s' "$SHORT_SHA" | tr -cd '[:xdigit:]' | cut -c1-12)"
+          SAFE_ACTOR="$(printf '%s' "$ACTOR" | tr -cd '[:alnum:]-')"
+
+          if [[ -z "$SAFE_REPO_NAME" || -z "$SAFE_SHORT_SHA" ]]; then
+            echo "Invalid payload after sanitization" >&2
+            exit 1
+          fi
 
-          BRANCH="sync/${REPO_NAME}/${SHORT_SHA}"
-          PR_TITLE="chore(${REPO_NAME}): sync ${SHORT_SHA}"
+          BRANCH="sync/${SAFE_REPO_NAME}/${SAFE_SHORT_SHA}"
+          PR_TITLE="chore(${SAFE_REPO_NAME}): sync ${SAFE_SHORT_SHA}"
@@
-          Synced from ${REPO_NAME}@${SHORT_SHA}
+          Synced from ${SAFE_REPO_NAME}@${SAFE_SHORT_SHA}
@@
-          Co-authored-by: ${ACTOR} <${ACTOR}@users.noreply.github.com>"
+          Co-authored-by: ${SAFE_ACTOR} <${SAFE_ACTOR}@users.noreply.github.com>"

Also applies to: 52-56

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/on-child-update.yml around lines 34 - 40, Sanitize and
validate payload-derived vars before using them: ensure REPO_NAME, SHORT_SHA,
and ACTOR are normalized (strip/replace unsafe chars, e.g. allow only
[A-Za-z0-9._-], replace others with '-') and truncate to safe lengths, build
BRANCH using the sanitized REPO_NAME and sanitized SHORT_SHA; for SHORT_SHA
prefer a real git short hash (e.g., git rev-parse --short HEAD) and only fall
back to a numeric timestamp if no commit hash is available, and ensure PR_TITLE
uses the sanitized values; when constructing Co-authored-by trailers, derive a
safe email/name from sanitized ACTOR (escape/omit characters that would create
invalid email syntax) and format it as "Co-authored-by: Name
<name@users.noreply.github.com>" so git commit metadata cannot be malformed.

@mateodelnorte mateodelnorte Mar 26, 2026

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 64031a6.

@coderabbitai coderabbitai Bot Mar 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧩 Analysis chain

@mateodelnorte, thanks for the fix! I'll verify the changes in commit 64031a6 to confirm the sanitization is in place.

🏁 Script executed:

#!/bin/bash
git show 64031a6 -- .github/workflows/on-child-update.yml 2>/dev/null || git log --oneline | head -10

Length of output: 3429

@mateodelnorte, thanks for making those improvements! Looking at commit 64031a6, I can confirm several concerns were resolved:

  • ✅ ACTOR removed entirely, eliminating the malformed Co-authored-by email issue.
  • ✅ Bot email updated to use the correct numeric ID (41898282+github-actions[bot]@users.noreply.github.com).
  • ✅ Orphaned branch cleanup added.
  • ✅ Failed PR creation now deletes the orphaned branch and exits with an error.

However, the core sanitization concern for REPO_NAME and SHORT_SHA was not addressed — these values are still used directly in git ref names and PR titles without stripping characters unsafe for git refs:

# Still present — no character sanitization:
REPO_NAME="${REPO_NAME:-unknown}"
SHORT_SHA="${SHORT_SHA:-$(date +%s | tail -c 8)}"

BRANCH="sync/${REPO_NAME}/${SHORT_SHA}"   # unsanitized
PR_TITLE="chore(${REPO_NAME}): sync ${SHORT_SHA}"  # unsanitized

A malformed repo_name in the payload (e.g., containing spaces, .., or control characters) could still cause git push to fail with a confusing error. If you trust that all dispatching child repos will always send well-formed values, this may be an acceptable risk — but if you'd like to close the gap, the sanitization lines from the original suggestion would still apply:

SAFE_REPO_NAME="$(printf '%s' "$REPO_NAME" | tr '[:upper:]' '[:lower:]' | sed -E 's#[^a-z0-9._/-]+#-#g; s#(^/+|/+$)##g')"
SAFE_SHORT_SHA="$(printf '%s' "$SHORT_SHA" | tr -cd '[:xdigit:]' | cut -c1-12)"

Would you like to add these, or is the current trust boundary acceptable for this workflow?

# Check for existing branch
if git ls-remote --exit-code origin "refs/heads/$BRANCH" >/dev/null 2>&1; then
EXISTING_PR=$(gh pr list --head "$BRANCH" --base main --json url --jq '.[0].url // empty')
if [[ -n "$EXISTING_PR" ]]; then
echo "PR already exists: $EXISTING_PR"
gh pr merge "$EXISTING_PR" --auto --squash --delete-branch || true
exit 0
fi
# Branch exists but no PR — previous run pushed but failed to create PR.
# Delete the orphaned branch and recreate cleanly.
echo "Found orphaned branch $BRANCH (no PR) — deleting and recreating"
delete_output=$(git push origin --delete "$BRANCH" 2>&1) || {
if git ls-remote --exit-code origin "refs/heads/$BRANCH" >/dev/null 2>&1; then
echo "Failed to delete orphaned branch: $delete_output"
exit 1
fi
}
fi

git checkout -b "$BRANCH"
git commit --allow-empty -m "$PR_TITLE"

push_output=$(git push -u origin "$BRANCH" 2>&1) || {
if echo "$push_output" | grep -qE "(non-fast-forward|already exists|fetch first)"; then
echo "Push failed due to race condition — another run likely handled this"
exit 0
fi
echo "Push failed unexpectedly: $push_output"
exit 1
}

PR_URL=$(gh pr create \
--title "$PR_TITLE" \
--body "Synced from \`${REPO_NAME}@${SHORT_SHA}\`" \
--head "$BRANCH" \
--base main) || {
echo "PR creation failed — deleting orphaned branch"
git push origin --delete "$BRANCH" || true
exit 1
}
Comment on lines +71 to +79

@greptile-apps greptile-apps Bot Mar 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Orphaned branch silently swallows future retries

If gh pr create fails for any non-race reason (GitHub API error, rate limit, etc.), the branch sync/${REPO_NAME}/${SHORT_SHA} is left on the remote with no PR. On any subsequent dispatch for the same SHA, execution reaches line 42, finds the branch exists, queries for an open PR, gets nothing back, and falls through to the main path — where the push at line 58 immediately fails and the run exits cleanly at line 60. The update is silently and permanently dropped with no recovery path.

This is distinct from the intended race-condition case: the concurrency group serialises runs, so true race losers are fine. The problem is gh pr create failing for unrelated reasons after the push has already succeeded.

A safer pattern would be to check for the branch+no-PR case explicitly and attempt PR creation again, rather than treating it as a completed race:

# Inside the existing-branch block
EXISTING_PR=$(gh pr list --head "$BRANCH" --base main --json url --jq '.[0].url // empty')
if [[ -n "$EXISTING_PR" ]]; then
  echo "PR already exists: $EXISTING_PR"
  gh pr merge "$EXISTING_PR" --auto --squash --delete-branch || true
  exit 0
fi
# Branch exists but no PR — previous run pushed but failed to create the PR; fall through to create it
echo "Branch exists but no PR found — attempting PR creation"
git fetch origin "$BRANCH"
git checkout "$BRANCH"
# skip the commit/push steps and jump straight to gh pr create

Alternatively, delete the orphaned branch and start fresh so the normal flow can proceed.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: .github/workflows/on-child-update.yml
Line: 63-70

Comment:
**Orphaned branch silently swallows future retries**

If `gh pr create` fails for any non-race reason (GitHub API error, rate limit, etc.), the branch `sync/${REPO_NAME}/${SHORT_SHA}` is left on the remote with no PR. On any subsequent dispatch for the *same* SHA, execution reaches line 42, finds the branch exists, queries for an open PR, gets nothing back, and falls through to the main path — where the push at line 58 immediately fails and the run exits cleanly at line 60. The update is silently and permanently dropped with no recovery path.

This is distinct from the intended race-condition case: the concurrency group serialises runs, so true race losers are fine. The problem is `gh pr create` failing for unrelated reasons *after* the push has already succeeded.

A safer pattern would be to check for the branch+no-PR case explicitly and attempt PR creation again, rather than treating it as a completed race:

```
# Inside the existing-branch block
EXISTING_PR=$(gh pr list --head "$BRANCH" --base main --json url --jq '.[0].url // empty')
if [[ -n "$EXISTING_PR" ]]; then
  echo "PR already exists: $EXISTING_PR"
  gh pr merge "$EXISTING_PR" --auto --squash --delete-branch || true
  exit 0
fi
# Branch exists but no PR — previous run pushed but failed to create the PR; fall through to create it
echo "Branch exists but no PR found — attempting PR creation"
git fetch origin "$BRANCH"
git checkout "$BRANCH"
# skip the commit/push steps and jump straight to gh pr create
```

Alternatively, delete the orphaned branch and start fresh so the normal flow can proceed.

How can I resolve this? If you propose a fix, please make it concise.

@mateodelnorte mateodelnorte Mar 26, 2026

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 64031a6.


gh pr merge "$PR_URL" --auto --squash --delete-branch || true
echo "PR ready: $PR_URL"
Comment thread
coderabbitai[bot] marked this conversation as resolved.