From 3beb3591e39eb552452bee9087cde90050613298 Mon Sep 17 00:00:00 2001 From: Marcus Burghardt Date: Tue, 28 Jul 2026 10:04:22 +0200 Subject: [PATCH 1/2] fix: use glob patterns and include-based scoping for safe-settings Fix two issues in the safe-settings restrictedRepos configuration: 1. deployment-settings.yml used regex-style patterns (^repo$) but safe-settings uses glob/minimatch for pattern matching, not regex. The anchored patterns never matched any repo names, which would cause an unscoped full sync to skip all repos. Replace with plain names that minimatch handles correctly for exact matching. 2. The scoped deployment-settings generation (repos input) built a restrictedRepos.exclude list from peribolos.yaml. Repos not in peribolos (e.g. nunya, roadmap) were missing from the exclude list, causing safe-settings to process repos it should not manage. Replace with a restrictedRepos.include allowlist built directly from the target repos input, matching the approach used by the base deployment-settings.yml. Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: Marcus Burghardt --- .github/workflows/safe_settings_sync.yml | 40 +++++++++--------------- safe-settings/deployment-settings.yml | 26 +++++++-------- 2 files changed, 28 insertions(+), 38 deletions(-) diff --git a/.github/workflows/safe_settings_sync.yml b/.github/workflows/safe_settings_sync.yml index d251ee0..06e5c91 100644 --- a/.github/workflows/safe_settings_sync.yml +++ b/.github/workflows/safe_settings_sync.yml @@ -63,38 +63,28 @@ jobs: run: | echo "Scoping safe-settings to repos: $TARGET_REPOS" - # Read all repo names from peribolos.yaml - ALL_REPOS=$(yq -r '.orgs[].repos | keys[]' peribolos.yaml) - - # Build the exclude list: all repos EXCEPT the target ones - EXCLUDE_LIST="" - for repo in $ALL_REPOS; do - SKIP=false - IFS=',' read -ra TARGETS <<< "$TARGET_REPOS" - for target in "${TARGETS[@]}"; do - target=$(echo "$target" | xargs) # trim whitespace - if [ "$repo" = "$target" ]; then - SKIP=true - break - fi - done - if [ "$SKIP" = "false" ]; then - EXCLUDE_LIST="${EXCLUDE_LIST} - ${repo}"$'\n' - fi + # Build an include (allowlist) from the target repos. + # Uses the same format as deployment-settings.yml: plain names + # matched by safe-settings via glob/minimatch (not regex). + # + # Previous approach used an exclude list built from + # peribolos.yaml, which missed repos not in peribolos + # (e.g. nunya, roadmap) causing safe-settings to process + # repos it should not manage. + INCLUDE_LIST="" + IFS=',' read -ra TARGETS <<< "$TARGET_REPOS" + for target in "${TARGETS[@]}"; do + target=$(echo "$target" | xargs) # trim whitespace + INCLUDE_LIST="${INCLUDE_LIST} - ${target}"$'\n' done - # Always exclude the admin repo - EXCLUDE_LIST="${EXCLUDE_LIST} - .github"$'\n' - EXCLUDE_LIST="${EXCLUDE_LIST} - admin"$'\n' - EXCLUDE_LIST="${EXCLUDE_LIST} - safe-settings"$'\n' - # Write scoped deployment-settings.yml (preserving validators # from the original file) { echo "# Auto-generated: scoped to repos: $TARGET_REPOS" echo "restrictedRepos:" - echo " exclude:" - echo -n "$EXCLUDE_LIST" + echo " include:" + echo -n "$INCLUDE_LIST" # Append validators from original file echo "" yq -r 'del(.restrictedRepos) | select(. != null)' \ diff --git a/safe-settings/deployment-settings.yml b/safe-settings/deployment-settings.yml index 74c262e..020e4eb 100644 --- a/safe-settings/deployment-settings.yml +++ b/safe-settings/deployment-settings.yml @@ -9,21 +9,21 @@ # including the .github admin repo (avoids circular dependency) # and any repos not onboarded to safe-settings. # -# IMPORTANT: safe-settings uses regex matching for restrictedRepos. -# Entries must be anchored with ^ and $ for exact matching, otherwise -# "complytime" would also match "complytime-demos", etc. +# safe-settings uses glob/minimatch for pattern matching (NOT regex). +# Plain names match exactly. Use * for wildcards (e.g. "core-*"). +# See: https://github.com/github/safe-settings#restrictedrepos restrictedRepos: include: - - "^complyctl$" - - "^community$" - - "^complypack$" - - "^complytime$" - - "^complytime-collector-components$" - - "^complytime-demos$" - - "^complytime-policies$" - - "^complytime-providers$" - - "^org-infra$" - - "^website$" + - complyctl + - community + - complypack + - complytime + - complytime-collector-components + - complytime-demos + - complytime-policies + - complytime-providers + - org-infra + - website # Config validators run against each config entry before applying. # Return true to allow, false to reject. From bada23f5b2da645f70fc589f04408e21c33d859d Mon Sep 17 00:00:00 2001 From: Marcus Burghardt Date: Tue, 28 Jul 2026 10:33:51 +0200 Subject: [PATCH 2/2] fix: work around check_suite crash in safe-settings full-sync mode safe-settings v2.1.18 crashes in full-sync mode (GHA/CLI) with: TypeError: Cannot read properties of undefined (reading "check_suite") The handleResults function in nop mode unconditionally accesses payload.check_run.check_suite, which only exists in the webhook flow. The sync itself completes successfully; only the Check Run reporting fails. Replace "npm run full-sync" with a patched script that catches the check_suite TypeError specifically and treats it as non-fatal, while preserving error handling for all other failure modes. This is a temporary workaround. The upstream fix is tracked at: - Issue: github-community-projects/safe-settings#818 - Fix PR: github-community-projects/safe-settings#1018 - Search: TODO(safe-settings-818) in the workflow file Update MAINTAINING.md troubleshooting section with the workaround details and links for tracking when to remove it. Assisted-by: OpenCode (claude-opus-4-6) Signed-off-by: Marcus Burghardt --- .github/workflows/safe_settings_sync.yml | 60 +++++++++++++++++++++++- MAINTAINING.md | 16 +++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/.github/workflows/safe_settings_sync.yml b/.github/workflows/safe_settings_sync.yml index 06e5c91..c0c41a0 100644 --- a/.github/workflows/safe_settings_sync.yml +++ b/.github/workflows/safe_settings_sync.yml @@ -27,7 +27,8 @@ name: Safe Settings Sync description: >- safe-settings commit SHA or tag to use. Default is 2.1.18 (last version compatible with Probot v13). - See upstream issue github/safe-settings#955 for Probot v14 status. + Includes workaround for check_suite bug (#818) in full-sync + mode. See TODO(safe-settings-818) in the sync step. required: false type: string default: '594f3c706de6c4ddafb1a86dfa7468f19337e54f' @@ -127,4 +128,59 @@ jobs: if [ "$FULL_SYNC_NOP" = "true" ]; then echo "=== DRY-RUN MODE: Showing what would change ===" fi - npm run full-sync + # --- Workaround for upstream check_suite bug --- + # + # safe-settings v2.1.18 crashes in full-sync mode because + # handleResults accesses payload.check_suite which is + # undefined outside the webhook flow. The sync itself + # completes; only the Check Run reporting fails. + # + # Upstream issue: github-community-projects/safe-settings#818 + # Upstream fix: github-community-projects/safe-settings#1018 + # + # TODO(safe-settings-818): Remove this patched script and + # revert to 'npm run full-sync' once upstream PR #1018 is + # merged and we pin to the fixed version. + # + # --- + cat > full-sync-patched.js << 'ENDPATCH' + const appFn = require('./') + const { FULL_SYNC_NOP } = require('./lib/env') + const { createProbot } = require('probot') + + async function performFullSync (appFn, nop) { + const probot = createProbot() + probot.log.info(`Starting full sync with NOP=${nop}`) + try { + const app = appFn(probot, {}) + const settings = await app.syncInstallation(nop) + if (settings.errors && settings.errors.length > 0) { + probot.log.error('Errors occurred during full sync.') + process.exit(1) + } + probot.log.info('Full sync completed successfully.') + } catch (error) { + // Workaround: github-community-projects/safe-settings#818 + // In full-sync mode (CLI/GHA), handleResults tries to + // access payload.check_run.check_suite which does not + // exist outside the webhook flow. The sync completes + // and results are logged; only the Check Run reporting + // fails. Safe to skip. + if (String(error).includes('check_suite')) { + probot.log.info( + 'check_suite reporting skipped (expected in full-sync mode). ' + + 'See: github-community-projects/safe-settings#818' + ) + return + } + process.stdout.write(`Unexpected error during full sync: ${error}\n`) + process.exit(1) + } + } + + performFullSync(appFn, FULL_SYNC_NOP).catch((error) => { + console.error('Fatal error during full sync:', error) + process.exit(1) + }) + ENDPATCH + node full-sync-patched.js diff --git a/MAINTAINING.md b/MAINTAINING.md index 7b542aa..cdd0b70 100644 --- a/MAINTAINING.md +++ b/MAINTAINING.md @@ -295,6 +295,22 @@ Common causes: - **safe-settings version issue**: If safe-settings behavior changes, check the pinned version in the workflow file. +### Known upstream workarounds + +The workflow includes a patched `full-sync.js` to work around a bug +in safe-settings where `handleResults` crashes in full-sync mode +because `payload.check_suite` is undefined outside the webhook flow +(the sync itself completes; only the Check Run reporting fails). + +- **Upstream issue**: + [github-community-projects/safe-settings#818](https://github.com/github-community-projects/safe-settings/issues/818) +- **Upstream fix PR**: + [github-community-projects/safe-settings#1018](https://github.com/github-community-projects/safe-settings/pull/1018) +- **Search tag**: `TODO(safe-settings-818)` in the workflow file + +Once upstream PR #1018 is merged and released, update the pinned +version and revert the patched script back to `npm run full-sync`. + ## GitHub Enterprise Hierarchy The complytime org is part of a GitHub Enterprise Cloud account. Enterprise admins can enforce policies and rulesets that layer on top of org-level settings. Understanding this hierarchy is important when managing settings with safe-settings.