From 3a934ea5e3e5c37847800074feef9e644a855ca9 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Tue, 8 Sep 2026 11:41:02 +0200 Subject: [PATCH 1/3] fix(workflows): restore complete repository drift checks --- .github/workflows/repository-drift-check.yaml | 19 +- AGENTS.md | 7 +- scripts/check-repository-drift.sh | 70 ++++++- tests/repository-drift.sh | 178 +++++++++++++++++- 4 files changed, 255 insertions(+), 19 deletions(-) diff --git a/.github/workflows/repository-drift-check.yaml b/.github/workflows/repository-drift-check.yaml index 0104b02..74b3712 100644 --- a/.github/workflows/repository-drift-check.yaml +++ b/.github/workflows/repository-drift-check.yaml @@ -13,12 +13,8 @@ name: 🔍 Repository drift check # belongs on a schedule and must never gate a merge. on: - # Dispatch only, until the App is granted Administration: Read-only - # (devantler-tech/.github#144). Without it the token request itself is refused - # with 422, so a scheduled run would fail every day on something no change in - # this repository can fix. Restore the schedule when the permission lands: - # schedule: - # - cron: "17 5 * * *" + schedule: + - cron: "17 5 * * *" workflow_dispatch: permissions: {} @@ -68,13 +64,10 @@ jobs: # used instead — it reaches this repository alone, and the org has a # private repository it could not see either way. # - # `administration: read` is what the merge-policy and feature fields - # need: GitHub returns allow_squash_merge, allow_merge_commit, - # allow_rebase_merge, allow_auto_merge, allow_update_branch, - # delete_branch_on_merge and web_commit_signoff_required only to a - # caller with administrative read. Under metadata alone the repository - # object simply omits them, and the check aborts — which is how this - # was found, on the first live run. + # An installation token's REST response can omit merge-policy fields + # even with administrative read. The comparison fills only missing + # declared settings from GraphQL and verifies both reads identify the + # same repository before comparing them. owner: ${{ github.repository_owner }} repositories: ${{ steps.targets.outputs.list }} permission-administration: read diff --git a/AGENTS.md b/AGENTS.md index 737863b..6c0ae8c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -131,10 +131,9 @@ Repo-specific watch-list for the daily engineer: `Repository` only PATCHes when it has a pending diff, so an `Observe`-only resource — or one whose writes GitHub rejects — reports `Synced=ReconcileSuccess` while its declaration is never applied. A `DRIFT` line is a real defect in one of those two shapes; fix the resource, never the live repo. - **It is `workflow_dispatch`-only** until the App holds `Administration: Read-only` - ([#144](https://github.com/devantler-tech/.github/issues/144)) — GitHub returns the merge-policy - and feature fields to no lesser scope, and requesting an ungranted permission fails the token step - outright. Restore the daily `schedule:` with the permission. + It runs daily at 05:17 UTC and on `workflow_dispatch`. The scoped App token reads repository + settings through REST and fills missing merge-policy fields through GraphQL. Both reads must + identify the same repository and visibility; missing fields or partial responses fail the check. - **`cd.yaml` is the publish path**, triggered on `v*` tags only; `ci.yaml` produces the PR-time required check. A red `cd.yaml` means the org-config OCI artifact didn't republish — investigate before assuming the live org is in sync. diff --git a/scripts/check-repository-drift.sh b/scripts/check-repository-drift.sh index 8f3ee3a..c615d80 100755 --- a/scripts/check-repository-drift.sh +++ b/scripts/check-repository-drift.sh @@ -85,6 +85,69 @@ readonly KEY_PROGRAM=' def api_key: {"homepageUrl": "homepage"}[.] // to_snake; ' +# GraphQL exposes these settings even when the REST repository object omits +# them. Only fill missing declared settings; an existing REST value, including +# false, remains authoritative for this read. +readonly SETTINGS_KEYS='["allow_auto_merge", "allow_squash_merge", "allow_merge_commit", + "allow_rebase_merge", "allow_update_branch", "delete_branch_on_merge", + "web_commit_signoff_required"]' +readonly SETTINGS_QUERY='query RepositorySettings($owner: String!, $name: String!) { + repository(owner: $owner, name: $name) { + id nameWithOwner isPrivate + allow_auto_merge: autoMergeAllowed + allow_squash_merge: squashMergeAllowed + allow_merge_commit: mergeCommitAllowed + allow_rebase_merge: rebaseMergeAllowed + allow_update_branch: allowUpdateBranch + delete_branch_on_merge: deleteBranchOnMerge + web_commit_signoff_required: webCommitSignoffRequired + } +}' + +complete_repository_settings() { + local entry="$1" live="$2" repo="$3" declared_settings missing settings + declared_settings="$(jq -c --argjson keys "$SETTINGS_KEYS" "$KEY_PROGRAM"' + [.declared | keys[] | api_key | select(. as $key | $keys | index($key))] + ' <<<"$entry")" || abort "failed to identify declared settings for '$repo'" + + jq -e --argjson keys "$declared_settings" ' + . as $live | all($keys[]; . as $key | + ($live | has($key) | not) or ($live[$key] | type == "boolean")) + ' >/dev/null <<<"$live" || abort "live repository settings for '$repo' are not booleans" + missing="$(jq -c --argjson keys "$declared_settings" ' + . as $live | [$keys[] | select(. as $key | $live | has($key) | not)] + ' <<<"$live")" || abort "failed to identify missing settings for '$repo'" + + # Fixture mode stays offline. Missing fixture fields still reach the ordinary + # unmapped-field rejection below; transport tests use a fake gh executable. + if [[ "$missing" == '[]' || -n "$live_dir" ]]; then + printf '%s\n' "$live" + return + fi + + jq -e ' + (.node_id | type == "string" and length > 0) and + (.full_name | type == "string" and length > 0) and + (.private | type == "boolean") + ' >/dev/null <<<"$live" || abort "REST repository identity for '$repo' is incomplete" + settings="$(gh api graphql -f query="$SETTINGS_QUERY" -f owner="$owner" -f name="$repo")" || + abort "failed to read GraphQL repository settings for '$repo'" + # Bind both reads to the same immutable repository and canonical name, also + # rejecting a visibility change before any values could enter public logs. + # Partial GraphQL data must never conceal an API error or a missing setting. + jq -e --argjson live "$live" --argjson keys "$SETTINGS_KEYS" ' + ((has("errors") | not) or .errors == null or .errors == []) and + (.data.repository | type == "object") and + (.data.repository.id == $live.node_id) and + (.data.repository.nameWithOwner == $live.full_name) and + (.data.repository.isPrivate == $live.private) and + (.data.repository as $settings | all($keys[]; $settings[.] | type == "boolean")) + ' >/dev/null <<<"$settings" || abort "GraphQL repository settings for '$repo' are incomplete or mismatched" + jq -c --argjson settings "$settings" --argjson missing "$missing" ' + reduce $missing[] as $key (. ; .[$key] = $settings.data.repository[$key]) + ' <<<"$live" || abort "failed to combine repository settings for '$repo'" +} + drift_found=0 while IFS= read -r entry; do @@ -111,7 +174,12 @@ while IFS= read -r entry; do # flag the sensitivity cannot be judged, so it aborts rather than guessing. jq -e 'has("private")' >/dev/null <<<"$live" || abort "live state for '$repo' has no 'private' flag, so its findings cannot be safely printed" - is_private="$(jq -r 'if .private == true then "true" else "false" end' <<<"$live")" + jq -e '.private | type == "boolean"' >/dev/null <<<"$live" || + abort "live state for '$repo' has a non-boolean 'private' flag, so its findings cannot be safely printed" + is_private="$(jq -r '.private' <<<"$live")" + + live="$(complete_repository_settings "$entry" "$live" "$repo")" || + abort "could not complete repository settings for '$repo'" # A declared field with no counterpart on the live object means the mapping # is wrong or the API changed shape. Silently skipping it would let a whole diff --git a/tests/repository-drift.sh b/tests/repository-drift.sh index c666a4a..4833d1a 100644 --- a/tests/repository-drift.sh +++ b/tests/repository-drift.sh @@ -228,4 +228,180 @@ expect_status "$work/collapsed" 2 "a collapsed render" grep -Fq "collapsed to" "$work/collapsed/stderr" || fail "a collapsed render must say so" -echo "repository-drift: OK — agreement, drift, set-compare, external-name, private redaction and four fail-closed paths" +# Exercise the real gh transport path, including the REST omissions observed +# with an installation token. The fake accepts only the expected read requests. +transport="$work/transport" +build_fixture "$transport" +mkdir -p "$transport/bin" "$transport/graphql" +yq '.spec.forProvider *= {"allowAutoMerge": true, "allowRebaseMerge": false, "allowUpdateBranch": true, "deleteBranchOnMerge": true}' \ + "$transport/render.yaml" >"$transport/render.new" +mv "$transport/render.new" "$transport/render.yaml" +for file in "$transport"/live/*.json; do + jq '. + {node_id: ("R_" + .name), full_name: ("devantler-tech/" + .name), + allow_auto_merge: true, allow_rebase_merge: false, allow_update_branch: true, + delete_branch_on_merge: true}' "$file" >"$file.new" + mv "$file.new" "$file" + jq '{data: {repository: { + id: .node_id, nameWithOwner: .full_name, isPrivate: .private, + allow_auto_merge, allow_squash_merge, allow_merge_commit, allow_rebase_merge, + allow_update_branch, delete_branch_on_merge, web_commit_signoff_required + }}}' "$file" >"$transport/graphql/$(basename "$file")" +done +cat >"$transport/bin/gh" <<'EOF' +#!/usr/bin/env bash +set -euo pipefail +[[ "$1" == api ]] || exit 91 +if [[ "$2" == graphql ]]; then + shift 2 + owner="" name="" query="" + while [[ "$#" -gt 0 ]]; do + [[ "$1" == -f || "$1" == -F ]] || exit 92 + case "$2" in + owner=*) owner="${2#owner=}" ;; + name=*) name="${2#name=}" ;; + query=*) query="${2#query=}" ;; + *) exit 93 ;; + esac + shift 2 + done + [[ "$owner" == devantler-tech && "$name" == fixture-repo-* ]] || exit 94 + for field in autoMergeAllowed squashMergeAllowed mergeCommitAllowed rebaseMergeAllowed \ + allowUpdateBranch deleteBranchOnMerge webCommitSignoffRequired nameWithOwner isPrivate; do + [[ "$query" == *"$field"* ]] || exit 95 + done + echo "$name" >>"$DRIFT_GH_FIXTURES/graphql-calls" + [[ ! -e "$DRIFT_GH_FIXTURES/graphql-error" ]] || exit 96 + cat "$DRIFT_GH_FIXTURES/graphql/$name.json" +else + [[ "$#" == 2 && "$2" == repos/devantler-tech/fixture-repo-* ]] || exit 97 + cat "$DRIFT_GH_FIXTURES/live/${2##*/}.json" +fi +EOF +chmod +x "$transport/bin/gh" + +expect_transport_status() { + local dir="$1" want="$2" what="$3" got=0 + PATH="$dir/bin:$PATH" DRIFT_GH_FIXTURES="$dir" \ + REPOSITORY_DRIFT_OWNER=devantler-tech \ + REPOSITORY_DRIFT_RENDER="$dir/render.yaml" REPOSITORY_DRIFT_LIVE_DIR= \ + bash "$check" >"$dir/stdout" 2>"$dir/stderr" || got=$? + [[ "$got" -eq "$want" ]] || { + cat "$dir/stdout" "$dir/stderr" >&2 + fail "$what expected exit $want, got $got" + } +} + +expect_transport_status "$transport" 0 "complete REST response" +[[ ! -e "$transport/graphql-calls" ]] || fail "complete REST data must not query GraphQL" + +missing_settings="$work/missing-settings" +cp -R "$transport" "$missing_settings" +for file in "$missing_settings"/live/*.json; do + jq 'del(.allow_auto_merge, .allow_squash_merge, .allow_merge_commit, + .allow_rebase_merge, .allow_update_branch, .delete_branch_on_merge, + .web_commit_signoff_required)' "$file" >"$file.new" + mv "$file.new" "$file" +done +expect_transport_status "$missing_settings" 0 "REST omits settings, GraphQL supplies them" +[[ "$(wc -l <"$missing_settings/graphql-calls" | tr -d ' ')" == "$FIXTURE_REPOS" ]] || + fail "every incomplete REST response must get one scoped GraphQL read" +grep -Fxq "$RENAMED_EXTERNAL" "$missing_settings/graphql-calls" || + fail "GraphQL lookup must use the declared external repository name" + +# Every mapped field must still detect a real divergence, including false values. +for pair in allow_auto_merge:allowAutoMerge allow_squash_merge:allowSquashMerge \ + allow_merge_commit:allowMergeCommit allow_rebase_merge:allowRebaseMerge \ + allow_update_branch:allowUpdateBranch delete_branch_on_merge:deleteBranchOnMerge \ + web_commit_signoff_required:webCommitSignoffRequired; do + dir="$work/graphql-drift-${pair%%:*}" + cp -R "$missing_settings" "$dir" + jq --arg field "${pair%%:*}" '.data.repository[$field] |= not' \ + "$dir/graphql/fixture-repo-1.json" >"$dir/changed.json" + mv "$dir/changed.json" "$dir/graphql/fixture-repo-1.json" + expect_transport_status "$dir" 1 "GraphQL drift in ${pair%%:*}" + grep -Fq "DRIFT fixture-repo-1.${pair#*:}:" "$dir/stdout" || + fail "GraphQL drift must name the mapped declared field" +done + +dir="$work/rest-false-wins" +cp -R "$missing_settings" "$dir" +jq '.allow_merge_commit = false' "$dir/live/fixture-repo-1.json" >"$dir/changed.json" +mv "$dir/changed.json" "$dir/live/fixture-repo-1.json" +jq '.data.repository.allow_merge_commit = true' "$dir/graphql/fixture-repo-1.json" >"$dir/changed.json" +mv "$dir/changed.json" "$dir/graphql/fixture-repo-1.json" +expect_transport_status "$dir" 0 "present false REST setting wins over GraphQL" + +for invalid in wrong-id wrong-name changed-privacy null-repository partial-error false-errors \ + missing-boolean null-boolean string-boolean malformed-json transport-error; do + dir="$work/graphql-$invalid" + cp -R "$missing_settings" "$dir" + file="$dir/graphql/fixture-repo-1.json" + case "$invalid" in + wrong-id) filter='.data.repository.id = "R_different"' ;; + wrong-name) filter='.data.repository.nameWithOwner = "another/repository"' ;; + changed-privacy) filter='.data.repository.isPrivate = true' ;; + null-repository) filter='.data.repository = null' ;; + partial-error) filter='.errors = [{message: "partial response"}]' ;; + false-errors) filter='.errors = false' ;; + missing-boolean) filter='del(.data.repository.allow_auto_merge)' ;; + null-boolean) filter='.data.repository.allow_auto_merge = null' ;; + string-boolean) filter='.data.repository.allow_auto_merge = "true"' ;; + malformed-json) printf '{' >"$file"; filter='' ;; + transport-error) touch "$dir/graphql-error"; filter='' ;; + esac + if [[ -n "$filter" ]]; then + jq "$filter" "$file" >"$dir/changed.json" + mv "$dir/changed.json" "$file" + fi + expect_transport_status "$dir" 2 "invalid GraphQL response: $invalid" +done + +for invalid in missing-id missing-name null-setting string-setting null-privacy string-privacy; do + dir="$work/rest-$invalid" + cp -R "$missing_settings" "$dir" + case "$invalid" in + missing-id) filter='del(.node_id)' ;; + missing-name) filter='del(.full_name)' ;; + null-setting) filter='.allow_merge_commit = null' ;; + string-setting) filter='.allow_merge_commit = "false"' ;; + null-privacy) filter='.private = null' ;; + string-privacy) filter='.private = "false"' ;; + esac + jq "$filter" "$dir/live/fixture-repo-1.json" >"$dir/changed.json" + mv "$dir/changed.json" "$dir/live/fixture-repo-1.json" + expect_transport_status "$dir" 2 "invalid REST response: $invalid" +done + +dir="$work/rest-complete-invalid-privacy" +cp -R "$transport" "$dir" +jq '.private = "false" | .description = "sensitive live description"' \ + "$dir/live/fixture-repo-1.json" >"$dir/changed.json" +mv "$dir/changed.json" "$dir/live/fixture-repo-1.json" +expect_transport_status "$dir" 2 "complete REST response with malformed visibility" +! grep -Fq 'sensitive live description' "$dir/stdout" "$dir/stderr" || + fail "malformed visibility must not disclose live values" + +dir="$work/graphql-private-drift" +cp -R "$missing_settings" "$dir" +jq '.private = true | .description = "sensitive live description"' \ + "$dir/live/fixture-repo-1.json" >"$dir/changed.json" +mv "$dir/changed.json" "$dir/live/fixture-repo-1.json" +jq '.data.repository.isPrivate = true | .data.repository.allow_auto_merge = false' \ + "$dir/graphql/fixture-repo-1.json" >"$dir/changed.json" +mv "$dir/changed.json" "$dir/graphql/fixture-repo-1.json" +expect_transport_status "$dir" 1 "private drift after GraphQL completion" +grep -Fq 'DRIFT fixture-repo-1.allowAutoMerge: values withheld' "$dir/stdout" || + fail "GraphQL completion must retain private-repository redaction" +! grep -Fq 'sensitive live description' "$dir/stdout" "$dir/stderr" || + fail "GraphQL completion must not disclose private live values" + +# Unknown declarations must still fail closed after known settings are filled. +dir="$work/graphql-unmapped" +cp -R "$missing_settings" "$dir" +yq '.spec.forProvider.inventedSetting = true' "$dir/render.yaml" >"$dir/render.new" +mv "$dir/render.new" "$dir/render.yaml" +expect_transport_status "$dir" 2 "unmapped declaration after GraphQL read" +grep -Fq 'inventedSetting but the live repository object has no "invented_setting" field' "$dir/stderr" || + fail "GraphQL completion must not hide unrelated missing fields" + +echo "repository-drift: OK — comparison, private redaction, REST/GraphQL settings and fail-closed reads" From 6a2927fef59ff091e565c317a54bc71967c76113 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Tue, 8 Sep 2026 11:46:35 +0200 Subject: [PATCH 2/3] fix(workflows): downscope repository drift token to metadata reads --- .github/workflows/repository-drift-check.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/repository-drift-check.yaml b/.github/workflows/repository-drift-check.yaml index 74b3712..2dcccec 100644 --- a/.github/workflows/repository-drift-check.yaml +++ b/.github/workflows/repository-drift-check.yaml @@ -70,7 +70,6 @@ jobs: # same repository before comparing them. owner: ${{ github.repository_owner }} repositories: ${{ steps.targets.outputs.list }} - permission-administration: read permission-metadata: read - name: 🔍 Compare declared and live repository settings From 836da65aac6ba46cacca7c1ca72245dc18406bea Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Tue, 8 Sep 2026 11:57:34 +0200 Subject: [PATCH 3/3] test(drift): quote the empty fixture override --- tests/repository-drift.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/repository-drift.sh b/tests/repository-drift.sh index 4833d1a..49ae448 100644 --- a/tests/repository-drift.sh +++ b/tests/repository-drift.sh @@ -283,7 +283,7 @@ expect_transport_status() { local dir="$1" want="$2" what="$3" got=0 PATH="$dir/bin:$PATH" DRIFT_GH_FIXTURES="$dir" \ REPOSITORY_DRIFT_OWNER=devantler-tech \ - REPOSITORY_DRIFT_RENDER="$dir/render.yaml" REPOSITORY_DRIFT_LIVE_DIR= \ + REPOSITORY_DRIFT_RENDER="$dir/render.yaml" REPOSITORY_DRIFT_LIVE_DIR="" \ bash "$check" >"$dir/stdout" 2>"$dir/stderr" || got=$? [[ "$got" -eq "$want" ]] || { cat "$dir/stdout" "$dir/stderr" >&2