Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 13 additions & 1 deletion .github/workflows/standards-deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,19 @@ jobs:
permissions:
contents: read # only to check out this repo; all cross-repo writes use the PAT below
env:
GH_TOKEN: ${{ secrets.ORG_SCORECARD_TOKEN }}
# Cross-repo write identity for the sweep. NOT ORG_SCORECARD_TOKEN — that is
# the audit identity (a fine-grained PAT with Contents: Read-only), so it
# cannot PUT stub content into consumer repos and every deploy 422'd
# (petry-projects/.github#864). GH_PAT_DON_PETRY (falling back to the legacy
# GH_PAT_WORKFLOWS) is the SAME identity dev-lead.yml / pr-auto-review.yml /
# initiative-driver.yml already use to author cross-repo PRs. Its login is
# `don-petry` (or the GH_PAT_WORKFLOWS owner) — deliberately NOT donpetry-bot
# — so the standards-sync PRs it opens can be approved by donpetry-bot via the
# pr-review agent and land through native auto-merge with no `--admin`.
# Least-privilege: needs only contents:write + pull_requests:write on the
# target repos (a GitHub App with those two scopes, non-author-blocked and not
# donpetry-bot, is an acceptable substitute).
GH_TOKEN: ${{ secrets.GH_PAT_DON_PETRY || secrets.GH_PAT_WORKFLOWS }}
TARGET_REPO: ${{ inputs.target_repo }}
TARGET_WORKFLOW: ${{ inputs.target_workflow }}
DRY_RUN: ${{ inputs.dry_run == true && 'true' || 'false' }}
Expand Down
74 changes: 62 additions & 12 deletions scripts/lib/standards-deploy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@
# default branch directly and NEVER merges or uses --admin. Instead it:
#
# 1. checks for an already-open PR on the sync branch (idempotent skip),
# 2. creates (or reuses) a sync branch off the repo's default branch,
# 3. PUTs the verbatim file content onto that branch via the Contents API,
# 4. opens a labeled pull request.
# 2. preflights that the token can write the repo (clear finding on a scope gap),
# 3. creates (or reuses) a sync branch off the repo's default branch,
# 4. PUTs the verbatim file content onto that branch via the Contents API,
# 5. opens a labeled pull request.
#
# Why a PR and not a direct push: a direct Contents-API push to the default
# branch is rejected (HTTP 409) on repos whose ruleset enforces required status
Expand Down Expand Up @@ -81,7 +82,28 @@ sd_deploy_files_via_pr() {
return 0
fi

# 2. Resolve the default branch and its tip SHA (branch point).
# 2. Preflight write check. The driver must run under a contents/PR-write
Comment thread
don-petry marked this conversation as resolved.
# identity; an audit/read-only token silently produces per-file 422s deep in
# step 5 (petry-projects/.github#864). Probe the repo permission set once, up
# front, so a token-scope gap surfaces as one clear finding, not opaque
# per-file put-failures. The probe's OWN errors (a transient/auth/404) are
# surfaced distinctly — they must NOT be misreported as "no write access".
# Repo-level push permission implies PR-create for a collaborator identity.
local can_push perm_err perm_rc
perm_err=$(mktemp)
can_push=$(gh api "repos/${repo}" --jq '.permissions.push // false' 2>"$perm_err") && perm_rc=0 || perm_rc=$?
if [ "$perm_rc" -ne 0 ]; then
local perm_msg; perm_msg=$(tr '\n' ' ' < "$perm_err"); rm -f "$perm_err"
echo "FAILED perm-probe-failed:${perm_msg}"
return 1
fi
rm -f "$perm_err"
if [ "$can_push" != "true" ]; then
echo "FAILED no-write-access"
return 1
fi

# 3. Resolve the default branch and its tip SHA (branch point).
local default_branch base_sha
default_branch=$(gh api "repos/${repo}" --jq '.default_branch' 2>/dev/null || echo "main")
base_sha=$(gh api "repos/${repo}/git/ref/heads/${default_branch}" \
Expand All @@ -91,7 +113,7 @@ sd_deploy_files_via_pr() {
return 1
fi

# 3. Create the sync branch; reuse it if a prior run already created it.
# 4. Create the sync branch; reuse it if a prior run already created it.
if ! gh api "repos/${repo}/git/refs" --method POST \
--raw-field "ref=refs/heads/${branch}" \
--raw-field "sha=${base_sha}" --silent 2>/dev/null; then
Expand All @@ -101,14 +123,39 @@ sd_deploy_files_via_pr() {
fi
fi

# 4. PUT each file verbatim onto the branch. The blob SHA is looked up per file
# 5. PUT each file verbatim onto the branch. The blob SHA is looked up per file
# on the branch so both create (absent → empty SHA) and update (drifted stub)
# work, whether the branch is fresh or reused.
local path local_file branch_sha encoded j
# work, whether the branch is fresh or reused. Errors are NOT swallowed: a
# read failure that yields an empty SHA would make the subsequent PUT omit
# `sha` and 422 against an existing file — the exact opaque failure in #864.
local path local_file branch_sha encoded j err_file get_rc get_msg put_msg
for (( i = 1; i < $#; i += 2 )); do
j=$(( i + 1 )); path="${!i}"; local_file="${!j}"

# Resolve the file's blob SHA on the branch, capturing stderr and exit code.
err_file=$(mktemp)
branch_sha=$(gh api "repos/${repo}/contents/${path}?ref=${branch}" \
--jq '.sha // ""' 2>/dev/null || true)
--jq '.sha // ""' 2>"$err_file") && get_rc=0 || get_rc=$?
if [ "$get_rc" -eq 0 ]; then
# GET succeeded → the file EXISTS on the branch, so its SHA must resolve.
# A blank SHA here means we cannot update it safely; a sha-less PUT would
# 422. Fail loudly rather than mask it.
if [ -z "$branch_sha" ]; then
rm -f "$err_file"
echo "FAILED sha-unresolved:${path}"
return 1
fi
else
get_msg=$(tr '\n' ' ' < "$err_file"); rm -f "$err_file"
if printf '%s' "$get_msg" | grep -q 'HTTP 404'; then
branch_sha="" # file absent on the branch → create path, no SHA
else
echo "FAILED contents-get-failed:${get_msg}"
return 1
fi
fi
rm -f "$err_file"

encoded=$(base64 -w 0 "$local_file" 2>/dev/null || base64 -b 0 "$local_file")

local put_args=(--method PUT
Expand All @@ -117,13 +164,16 @@ sd_deploy_files_via_pr() {
--raw-field "branch=${branch}")
[ -n "$branch_sha" ] && put_args+=(--raw-field "sha=${branch_sha}")

if ! gh api "repos/${repo}/contents/${path}" "${put_args[@]}" --silent 2>/dev/null; then
echo "FAILED put-failed"
err_file=$(mktemp)
if ! gh api "repos/${repo}/contents/${path}" "${put_args[@]}" --silent 2>"$err_file"; then
put_msg=$(tr '\n' ' ' < "$err_file"); rm -f "$err_file"
echo "FAILED put-failed:${put_msg}"
return 1
fi
rm -f "$err_file"
done

# 5. Ensure the label exists, then open the PR. `gh pr create --label` fails
# 6. Ensure the label exists, then open the PR. `gh pr create --label` fails
# outright if the label is absent, and consumer repos do not carry the
# standards-sync label by default. Create-if-missing (no --force, so an
# existing curated label is never clobbered); ignore the "already exists"
Expand Down
5 changes: 4 additions & 1 deletion scripts/standards-deploy-driver.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
# Overridable for tests:
# DEPLOY_SCRIPT Path to the deploy script (default: sibling in this dir).
#
# Requires GH_TOKEN with repo scope for cross-repo branch + PR creation.
# Requires GH_TOKEN to be a cross-repo WRITE identity — contents:write +
# pull_requests:write on the target repos (e.g. GH_PAT_DON_PETRY / the legacy
# GH_PAT_WORKFLOWS). The audit token (ORG_SCORECARD_TOKEN) is read-only for
# Contents and cannot deploy stubs — see petry-projects/.github#864.

set -euo pipefail

Expand Down
22 changes: 22 additions & 0 deletions standards/ci-standards.md
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,28 @@ open for that workflow. Deployment never edits a caller's `uses:`/`agent_ref`
pins by hand — a release is rolled out by moving the channel tag (see
[Reusable workflow versioning](#reusable-workflow-versioning--the-stable-channel)).

**Deploy identity (least-privilege).** The scheduled driver
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
([`standards-deploy.yml`](../.github/workflows/standards-deploy.yml)) writes stub content and
opens PRs **across the fleet**, so it must run under a cross-repo write identity —
`contents:write` + `pull_requests:write` on the target repos, and nothing more. It
uses `GH_PAT_DON_PETRY` (falling back to the legacy `GH_PAT_WORKFLOWS`), the same
identity `dev-lead.yml` / `pr-auto-review.yml` / `initiative-driver.yml` already
author cross-repo PRs with. It **must not** use `ORG_SCORECARD_TOKEN`: that is the
**audit** identity (a fine-grained PAT with **Contents: Read-only**), so its
content PUTs 422 and every deploy fails with an opaque `put-failed`
([#864](https://github.com/petry-projects/.github/issues/864)). The deploy identity
is also deliberately **not `donpetry-bot`** (it acts as `don-petry`): the PRs it
opens are authored by a distinct code-owner, so `donpetry-bot` can approve them via
the pr-review agent and they land through native auto-merge with **no `--admin`**. A
GitHub App carrying only those two scopes — non-author-blocked and not
`donpetry-bot` — is an acceptable substitute.

The deploy primitive also **preflights write access per repo** before touching any
branch and **never issues a sha-less content PUT against an existing file**: a
token-scope gap surfaces as a single `no-write-access` finding, and a genuine HTTP
error is reported with its status (e.g. `put-failed:… (HTTP 422)`) instead of being
swallowed.

#### Self-managing meta-repos: `SKIP_REPOS` + `SKIP_OVERRIDES`

`deploy-standard-workflows.sh` exempts a small set of **self-managing meta-repos**
Expand Down
112 changes: 107 additions & 5 deletions test/scripts/lib/standards-deploy.bats
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,40 @@ case "$sub" in
else
exit "${GH_REF_EXISTS_RC:-0}"
fi ;;
*contents*"?"ref=*) printf '%s' "${GH_FILE_SHA:-}" ;;
*contents*) exit "${GH_PUT_RC:-0}" ;;
repos/*) printf '%s' "${GH_DEFAULT_BRANCH:-main}" ;;
*contents*"?"ref=*)
# contents GET used to resolve a file's blob sha on the branch.
# GH_GET_RC!=0 -> the GET errors (prints GH_GET_ERR to stderr)
# GH_FILE_SHA unset -> file absent on the branch (404 create path)
# GH_FILE_SHA set (maybe
# empty) -> file present; echo the (maybe empty) sha
if [ "${GH_GET_RC:-0}" -ne 0 ]; then
printf '%s\n' "${GH_GET_ERR:-gh: Something went wrong (HTTP 500)}" >&2
exit "${GH_GET_RC}"
fi
if [ -z "${GH_FILE_SHA+x}" ]; then
printf '%s\n' 'gh: Not Found (HTTP 404)' >&2
exit 1
fi
printf '%s' "${GH_FILE_SHA}" ;;
*contents*)
# contents PUT. On failure surface an HTTP-status-bearing stderr message.
if [ "${GH_PUT_RC:-0}" -ne 0 ]; then
printf '%s\n' "${GH_PUT_ERR:-gh: sha was not supplied (HTTP 422)}" >&2
exit "${GH_PUT_RC}"
fi
exit 0 ;;
repos/*)
# repos/<repo>: the default-branch lookup AND the preflight write probe
# (--jq .permissions.push) both land here — differentiate on the jq arg.
if printf '%s' "$args" | grep -q 'permissions.push'; then
if [ -n "${GH_PERM_RC:-}" ] && [ "${GH_PERM_RC}" != "0" ]; then
printf '%s\n' "${GH_PERM_ERR:-gh: Not Found (HTTP 404)}" >&2
exit "${GH_PERM_RC}"
fi
printf '%s' "${GH_CAN_PUSH:-true}"
else
printf '%s' "${GH_DEFAULT_BRANCH:-main}"
fi ;;
esac
;;
esac
Expand Down Expand Up @@ -190,11 +221,13 @@ deploy() {
[ "$output" = "FAILED no-branch" ]
}

@test "fails when the PUT is rejected" {
@test "fails when the PUT is rejected and surfaces the real HTTP status" {
GH_PUT_RC="1"; export GH_PUT_RC
GH_PUT_ERR='gh: "sha" wasn'\''t supplied. (HTTP 422)'; export GH_PUT_ERR
run deploy
[ "$status" -ne 0 ]
[ "$output" = "FAILED put-failed" ]
[[ "$output" == FAILED\ put-failed:* ]]
[[ "$output" == *"HTTP 422"* ]]
}

@test "fails when the PR cannot be created" {
Expand All @@ -210,3 +243,72 @@ deploy() {
[ "$status" -ne 0 ]
[ "$output" = "FAILED missing-local-file" ]
}

# ---------------------------------------------------------------------------
# Fix 2 (petry-projects/.github#864): preflight write check + real error
# surfacing + never a sha-less PUT on an existing file.
# ---------------------------------------------------------------------------

# Preflight: a token without contents/PR write is reported as a clear finding,
# not an opaque per-file put-failed. No branch or PUT is attempted.
@test "fails loudly when the token lacks write access (preflight probe)" {
GH_CAN_PUSH="false"; export GH_CAN_PUSH
run deploy
[ "$status" -ne 0 ]
[ "$output" = "FAILED no-write-access" ]
! grep -q 'git/refs --method POST' "$GH_CALLS"
! grep -q 'contents/.* --method PUT' "$GH_CALLS"
}

# Preflight: an ERROR on the probe itself (transient / auth / 404) is surfaced
# distinctly with its HTTP status — NOT misreported as "no-write-access". No
# branch or PUT is attempted.
@test "preflight probe error is surfaced distinctly, not as no-write-access" {
GH_PERM_RC="1"; export GH_PERM_RC
GH_PERM_ERR='gh: Resource not accessible by integration (HTTP 403)'; export GH_PERM_ERR
run deploy
[ "$status" -ne 0 ]
[[ "$output" == FAILED\ perm-probe-failed:* ]]
[[ "$output" == *"HTTP 403"* ]]
[[ "$output" != *"no-write-access"* ]]
! grep -q 'git/refs --method POST' "$GH_CALLS"
! grep -q 'contents/.* --method PUT' "$GH_CALLS"
}

# Regression guard for the reported bug: when the stub file already exists on the
# branch, the update PUT must ALWAYS carry its blob sha — a sha-less PUT against
# an existing file is a guaranteed 422.
@test "existing-file update always supplies a sha in the PUT" {
GH_FILE_SHA="deadbeefsha"; export GH_FILE_SHA
run deploy
[ "$status" -eq 0 ]
local put_lines
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
put_lines=$(grep 'contents/.* --method PUT' "$GH_CALLS")
[ -n "$put_lines" ]
while IFS= read -r line; do
printf '%s' "$line" | grep -q -- '--raw-field sha=deadbeefsha' \
|| { echo "PUT without sha: $line"; return 1; }
done <<< "$put_lines"
}

# The contents GET must not swallow errors: a non-404 failure (e.g. a token-scope
# 403) is surfaced with its HTTP status, and no PUT is attempted.
@test "fails loudly when the contents GET errors with a non-404 status" {
GH_GET_RC="1"; export GH_GET_RC
GH_GET_ERR='gh: Resource not accessible by integration (HTTP 403)'; export GH_GET_ERR
run deploy
[ "$status" -ne 0 ]
[[ "$output" == FAILED\ contents-get-failed:* ]]
[[ "$output" == *"HTTP 403"* ]]
! grep -q 'contents/.* --method PUT' "$GH_CALLS"
}

# The file exists on the branch (GET 200) but its sha cannot be resolved: fail
# loudly rather than issue a sha-less PUT that is guaranteed to 422.
@test "fails loudly when an existing file's sha cannot be resolved" {
GH_FILE_SHA=""; export GH_FILE_SHA # present (GET 200) but empty sha
run deploy
[ "$status" -ne 0 ]
[[ "$output" == FAILED\ sha-unresolved:* ]]
! grep -q 'contents/.* --method PUT' "$GH_CALLS"
}
Loading