From 0607067e71baa997855c20fb16a9003703ff9545 Mon Sep 17 00:00:00 2001 From: James Hume Date: Wed, 22 Jul 2026 19:40:05 -0400 Subject: [PATCH 1/4] [docs] bootstrap hardened OB1 review gate --- .github/workflows/ob1-gate-v2.yml | 80 +++++++++++++++++++------- .github/workflows/ob1-pr-followups.yml | 40 ++++++++----- 2 files changed, 85 insertions(+), 35 deletions(-) diff --git a/.github/workflows/ob1-gate-v2.yml b/.github/workflows/ob1-gate-v2.yml index de229228a..fbb295a4f 100644 --- a/.github/workflows/ob1-gate-v2.yml +++ b/.github/workflows/ob1-gate-v2.yml @@ -15,16 +15,30 @@ name: OB1 PR Gate # Result: automated gate passes → human admin approves → merge allowed on: - pull_request: - types: [opened, synchronize, reopened, ready_for_review] + pull_request_target: + types: [opened, synchronize, reopened, ready_for_review, converted_to_draft] branches: [main] + workflow_dispatch: permissions: contents: read jobs: + event_guard: + name: OB1 Gate Event Guard + if: github.event_name != 'pull_request_target' + runs-on: ubuntu-latest + steps: + - name: Explain non-review gate run + env: + EVENT_NAME: ${{ github.event_name }} + run: | + echo "OB1 PR Gate received a ${EVENT_NAME} event." + echo "The contribution review only runs for pull_request_target events." + review: name: OB1 Review + if: github.event_name == 'pull_request_target' runs-on: ubuntu-latest steps: - name: Checkout PR head safely @@ -41,9 +55,20 @@ jobs: # Full fetch (no --depth) so the merge base with the PR head always # exists. A shallow base fetch can lose the merge base when main has # advanced, silently under-reporting the changed-file list. - git fetch origin "$BASE_REF" - # Fail closed if the merge base still cannot be resolved. - git merge-base "origin/$BASE_REF" HEAD >/dev/null + git fetch --no-tags --no-recurse-submodules origin \ + "+refs/heads/${BASE_REF}:refs/remotes/origin/${BASE_REF}" + + # Fail closed if the merge base still cannot be resolved, and prove + # that the resolved object is an ancestor of both sides of the diff. + merge_base=$(git merge-base "origin/${BASE_REF}" HEAD) + git cat-file -e "${merge_base}^{commit}" + git merge-base --is-ancestor "$merge_base" "origin/${BASE_REF}" + git merge-base --is-ancestor "$merge_base" HEAD + + # The PR checkout is untrusted. Always validate contribution metadata + # against the schema from the trusted base branch instead. + git show "origin/${BASE_REF}:.github/metadata.schema.json" \ + > /tmp/ob1-metadata.schema.json - name: Install metadata schema validator run: python3 -m pip install check-jsonschema @@ -58,16 +83,22 @@ jobs: # No fallback: if this diff fails, the gate fails closed instead of # silently checking only the last commit of a multi-commit PR. changed=$(git diff --name-only "origin/${BASE_REF}...HEAD") - echo "files<> "$GITHUB_OUTPUT" - echo "$changed" >> "$GITHUB_OUTPUT" - echo "EOF" >> "$GITHUB_OUTPUT" + files_delimiter="OB1_FILES_${GITHUB_RUN_ID}_${GITHUB_RUN_ATTEMPT}_${RANDOM}" + { + printf 'files<<%s\n' "$files_delimiter" + printf '%s\n' "$changed" + printf '%s\n' "$files_delimiter" + } >> "$GITHUB_OUTPUT" # Identify contribution folders (e.g., recipes/email-import/) # Exclude _template folders and top-level files - contrib_dirs=$(echo "$changed" | grep -E '^(recipes|schemas|dashboards|integrations|skills|primitives|extensions)/' | grep -v '/_template/' | cut -d'/' -f1,2 | sort -u || true) - echo "contrib_dirs<> "$GITHUB_OUTPUT" - echo "$contrib_dirs" >> "$GITHUB_OUTPUT" - echo "EOF" >> "$GITHUB_OUTPUT" + contrib_dirs=$(printf '%s\n' "$changed" | grep -E '^(recipes|schemas|dashboards|integrations|skills|primitives|extensions)/' | grep -v '/_template/' | cut -d'/' -f1,2 | sort -u || true) + dirs_delimiter="OB1_DIRS_${GITHUB_RUN_ID}_${GITHUB_RUN_ATTEMPT}_${RANDOM}" + { + printf 'contrib_dirs<<%s\n' "$dirs_delimiter" + printf '%s\n' "$contrib_dirs" + printf '%s\n' "$dirs_delimiter" + } >> "$GITHUB_OUTPUT" - name: Run review checks id: review @@ -171,7 +202,7 @@ jobs: # The PR title is never the authority for skipping checks — a # crafted "[docs] ..." title previously bypassed every check, # including the credential and binary scans, regardless of which - # files were changed. When no contribution folders are touched, the + # files were changed. When no contribution folder is touched, the # contribution checks are skipped but the security scans still run # and still block via the failed= output. if [ -z "$CONTRIB_DIRS" ]; then @@ -189,9 +220,12 @@ jobs: echo "secret_blocked=$secret_blocked" >> "$GITHUB_OUTPUT" comment="## OB1 PR Gate\n\nThis PR does not touch contribution folders (docs/governance change). Contribution checks were skipped, but credential and binary-file scans still ran.\n\n${results}\n${summary}" - echo "comment<> "$GITHUB_OUTPUT" - echo -e "$comment" >> "$GITHUB_OUTPUT" - echo "EOF" >> "$GITHUB_OUTPUT" + comment_delimiter="OB1_COMMENT_${GITHUB_RUN_ID}_${GITHUB_RUN_ATTEMPT}_${RANDOM}" + { + printf 'comment<<%s\n' "$comment_delimiter" + printf '%b\n' "$comment" + printf '%s\n' "$comment_delimiter" + } >> "$GITHUB_OUTPUT" exit 0 fi @@ -248,7 +282,7 @@ jobs: continue fi - if ! schema_output=$(check-jsonschema --schemafile .github/metadata.schema.json "$dir/metadata.json" 2>&1); then + if ! schema_output=$(check-jsonschema --schemafile /tmp/ob1-metadata.schema.json "$dir/metadata.json" 2>&1); then indented_output=$(printf '%s\n' "$schema_output" | sed 's/^/ /') rule3_detail="${rule3_detail} - \`$dir/metadata.json\` failed schema validation\n${indented_output}\n" rule3_pass=false @@ -551,7 +585,6 @@ jobs: # ─── Scope check ─── # Contribution PRs should only modify files in their own folder rule12_pass=true - rule12_detail="" out_of_scope="" while IFS= read -r f; do [ -z "$f" ] && continue @@ -589,7 +622,7 @@ jobs: while IFS= read -r link; do [ -z "$link" ] && continue # Strip anchor fragment - filepath=$(echo "$link" | sed 's/#.*//') + filepath=${link%%#*} [ -z "$filepath" ] && continue # Resolve relative to the contribution directory resolved="$dir/$filepath" @@ -669,9 +702,12 @@ jobs: comment="${comment}\n\n---\n\n### Post-Merge Tasks\n\n*These don't block merge — they're reminders for admins after this PR lands.*\n\n${reminders}" fi - echo "comment<> "$GITHUB_OUTPUT" - echo -e "$comment" >> "$GITHUB_OUTPUT" - echo "EOF" >> "$GITHUB_OUTPUT" + comment_delimiter="OB1_COMMENT_${GITHUB_RUN_ID}_${GITHUB_RUN_ATTEMPT}_${RANDOM}" + { + printf 'comment<<%s\n' "$comment_delimiter" + printf '%b\n' "$comment" + printf '%s\n' "$comment_delimiter" + } >> "$GITHUB_OUTPUT" - name: Write gate artifact env: diff --git a/.github/workflows/ob1-pr-followups.yml b/.github/workflows/ob1-pr-followups.yml index e22180dc2..f7b92bc10 100644 --- a/.github/workflows/ob1-pr-followups.yml +++ b/.github/workflows/ob1-pr-followups.yml @@ -16,8 +16,18 @@ permissions: id-token: write jobs: + ignore_non_pr_gate: + name: Ignore Non-PR Gate Run + if: github.event.workflow_run.event != 'pull_request' && github.event.workflow_run.event != 'pull_request_target' + runs-on: ubuntu-latest + steps: + - name: Explain skipped follow-up + run: | + echo "OB1 PR Follow-Ups only acts on pull_request or pull_request_target gate runs." + echo "Received upstream event: ${{ github.event.workflow_run.event }}" + followups: - if: github.event.workflow_run.event == 'pull_request' + if: github.event.workflow_run.event == 'pull_request' || github.event.workflow_run.event == 'pull_request_target' runs-on: ubuntu-latest concurrency: group: ob1-pr-followups-${{ github.event.workflow_run.pull_requests[0].number || github.event.workflow_run.id }} @@ -61,15 +71,17 @@ jobs: context_file="gate-artifact/ob1-review-context.json" summary_file="gate-artifact/ob1-review-summary.md" - echo "context_file=$context_file" >> "$GITHUB_OUTPUT" - echo "summary_file=$summary_file" >> "$GITHUB_OUTPUT" - echo "pr_number=$(jq -r '.pr_number' "$context_file")" >> "$GITHUB_OUTPUT" - echo "author_login=$(jq -r '.author_login' "$context_file")" >> "$GITHUB_OUTPUT" - echo "author_association=$(jq -r '.author_association' "$context_file")" >> "$GITHUB_OUTPUT" - echo "head_sha=$(jq -r '.head_sha' "$context_file")" >> "$GITHUB_OUTPUT" - echo "gate_failed=$(jq -r '.failed' "$context_file")" >> "$GITHUB_OUTPUT" - echo "secret_blocked=$(jq -r '.secret_blocked' "$context_file")" >> "$GITHUB_OUTPUT" - echo "is_draft=$(jq -r '.is_draft' "$context_file")" >> "$GITHUB_OUTPUT" + { + echo "context_file=$context_file" + echo "summary_file=$summary_file" + echo "pr_number=$(jq -r '.pr_number' "$context_file")" + echo "author_login=$(jq -r '.author_login' "$context_file")" + echo "author_association=$(jq -r '.author_association' "$context_file")" + echo "head_sha=$(jq -r '.head_sha' "$context_file")" + echo "gate_failed=$(jq -r '.failed' "$context_file")" + echo "secret_blocked=$(jq -r '.secret_blocked' "$context_file")" + echo "is_draft=$(jq -r '.is_draft' "$context_file")" + } >> "$GITHUB_OUTPUT" - name: Post gate summary and sync security label if: steps.artifact.outputs.found == 'true' @@ -167,9 +179,11 @@ jobs: over_quota=true fi - echo "trusted=$trusted" >> "$GITHUB_OUTPUT" - echo "open_pr_count=$open_pr_count" >> "$GITHUB_OUTPUT" - echo "over_quota=$over_quota" >> "$GITHUB_OUTPUT" + { + echo "trusted=$trusted" + echo "open_pr_count=$open_pr_count" + echo "over_quota=$over_quota" + } >> "$GITHUB_OUTPUT" - name: Sync maintainer triage label if: steps.artifact.outputs.found == 'true' From ebdeb6907224d2978e63f29a43731fb17ed983e7 Mon Sep 17 00:00:00 2001 From: James Hume Date: Wed, 22 Jul 2026 20:12:24 -0400 Subject: [PATCH 2/4] [docs] harden OB1 gate trust boundary --- .github/workflows/ob1-gate-v2.yml | 224 ++++++++++++++++++++----- .github/workflows/ob1-pr-followups.yml | 40 ++--- 2 files changed, 191 insertions(+), 73 deletions(-) diff --git a/.github/workflows/ob1-gate-v2.yml b/.github/workflows/ob1-gate-v2.yml index fbb295a4f..0a8fab2cb 100644 --- a/.github/workflows/ob1-gate-v2.yml +++ b/.github/workflows/ob1-gate-v2.yml @@ -41,11 +41,84 @@ jobs: if: github.event_name == 'pull_request_target' runs-on: ubuntu-latest steps: + - name: Install metadata schema validator in trusted runner temp + working-directory: ${{ runner.temp }} + env: + PYTHONSAFEPATH: "1" + run: python3 -P -m pip install --disable-pip-version-check check-jsonschema + - name: Checkout PR head safely uses: actions/checkout@v4 with: ref: refs/pull/${{ github.event.pull_request.number }}/head fetch-depth: 0 + persist-credentials: false + submodules: false + + - name: Reject unsafe PR tree before content inspection + run: | + set -euo pipefail + export LC_ALL=C + + # actions/checkout must not leave its authorization header in the + # untrusted checkout's local Git configuration. + if git config --local --get-regexp '^http\..*\.extraheader$' >/dev/null 2>&1; then + echo "Checkout credentials remained in .git/config; refusing inspection." + exit 1 + fi + + workspace_root=$(cd "$GITHUB_WORKSPACE" && pwd -P) + current_root=$(pwd -P) + if [ "$current_root" != "$workspace_root" ]; then + echo "Workflow is not running at the canonical workspace root." + exit 1 + fi + + # This repository intentionally contains only regular tracked files. + # Reject symlinks, gitlinks/submodules, control-character paths, and + # any path shape that could escape the canonical workspace before a + # parser or content-scanning tool sees the PR tree. + while IFS= read -r -d '' entry; do + if [[ "$entry" != *$'\t'* ]]; then + echo "Malformed index entry; refusing inspection." + exit 1 + fi + + metadata=${entry%%$'\t'*} + path=${entry#*$'\t'} + mode=${metadata%% *} + + case "$mode" in + 100644|100755) ;; + 120000) + printf 'Symlink rejected before inspection: %q\n' "$path" + exit 1 + ;; + 160000) + printf 'Gitlink rejected before inspection: %q\n' "$path" + exit 1 + ;; + *) + printf 'Unexpected tracked mode %s rejected: %q\n' "$mode" "$path" + exit 1 + ;; + esac + + if [[ "$path" =~ [[:cntrl:]] ]]; then + printf 'Control-character path rejected: %q\n' "$path" + exit 1 + fi + case "/$path/" in + *"/../"*|*"/./"*|"//"*|*"//"*) + printf 'Unsafe path rejected: %q\n' "$path" + exit 1 + ;; + esac + if [[ "$path" == /* ]]; then + printf 'Absolute path rejected: %q\n' "$path" + exit 1 + fi + done < <(git ls-files --stage -z) - name: Fetch base branch env: @@ -68,10 +141,7 @@ jobs: # The PR checkout is untrusted. Always validate contribution metadata # against the schema from the trusted base branch instead. git show "origin/${BASE_REF}:.github/metadata.schema.json" \ - > /tmp/ob1-metadata.schema.json - - - name: Install metadata schema validator - run: python3 -m pip install check-jsonschema + > "$RUNNER_TEMP/ob1-metadata.schema.json" - name: Get changed files id: changed @@ -79,26 +149,73 @@ jobs: BASE_REF: ${{ github.event.pull_request.base.ref }} run: | set -euo pipefail + export LC_ALL=C # Get files changed in this PR, diffed against the merge base. # No fallback: if this diff fails, the gate fails closed instead of # silently checking only the last commit of a multi-commit PR. - changed=$(git diff --name-only "origin/${BASE_REF}...HEAD") - files_delimiter="OB1_FILES_${GITHUB_RUN_ID}_${GITHUB_RUN_ATTEMPT}_${RANDOM}" - { - printf 'files<<%s\n' "$files_delimiter" - printf '%s\n' "$changed" - printf '%s\n' "$files_delimiter" - } >> "$GITHUB_OUTPUT" + changed_file="$RUNNER_TEMP/ob1-changed-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}.nul" + git diff --name-only -z "origin/${BASE_REF}...HEAD" > "$changed_file" + + changed_files=() + while IFS= read -r -d '' path; do + if [[ "$path" =~ [[:cntrl:]] ]]; then + printf 'Control-character changed path rejected: %q\n' "$path" + exit 1 + fi + case "/$path/" in + *"/../"*|*"/./"*|"//"*|*"//"*) + printf 'Unsafe changed path rejected: %q\n' "$path" + exit 1 + ;; + esac + if [[ "$path" == /* ]]; then + printf 'Absolute changed path rejected: %q\n' "$path" + exit 1 + fi + changed_files+=("$path") + done < "$changed_file" + + if [ "${#changed_files[@]}" -eq 0 ]; then + echo "No changed paths resolved for this pull request; failing closed." + exit 1 + fi + + # Compact JSON is a single-line GitHub output. No attacker-controlled + # multiline delimiter is used. + files_json=$(printf '%s\0' "${changed_files[@]}" | jq -Rsc 'split("\u0000")[:-1]') + printf 'files_json=%s\n' "$files_json" >> "$GITHUB_OUTPUT" # Identify contribution folders (e.g., recipes/email-import/) - # Exclude _template folders and top-level files - contrib_dirs=$(printf '%s\n' "$changed" | grep -E '^(recipes|schemas|dashboards|integrations|skills|primitives|extensions)/' | grep -v '/_template/' | cut -d'/' -f1,2 | sort -u || true) - dirs_delimiter="OB1_DIRS_${GITHUB_RUN_ID}_${GITHUB_RUN_ATTEMPT}_${RANDOM}" - { - printf 'contrib_dirs<<%s\n' "$dirs_delimiter" - printf '%s\n' "$contrib_dirs" - printf '%s\n' "$dirs_delimiter" - } >> "$GITHUB_OUTPUT" + # Exclude _template folders and top-level files. + contrib_dirs=() + for path in "${changed_files[@]}"; do + case "$path" in + recipes/*/*|schemas/*/*|dashboards/*/*|integrations/*/*|skills/*/*|primitives/*/*|extensions/*/*) + category=${path%%/*} + remainder=${path#*/} + folder=${remainder%%/*} + [ "$folder" = "_template" ] && continue + dir="$category/$folder" + already_seen=false + for existing_dir in "${contrib_dirs[@]}"; do + if [ "$existing_dir" = "$dir" ]; then + already_seen=true + break + fi + done + if [ "$already_seen" = false ]; then + contrib_dirs+=("$dir") + fi + ;; + esac + done + + if [ "${#contrib_dirs[@]}" -eq 0 ]; then + dirs_json='[]' + else + dirs_json=$(printf '%s\0' "${contrib_dirs[@]}" | jq -Rsc 'split("\u0000")[:-1]') + fi + printf 'contrib_dirs_json=%s\n' "$dirs_json" >> "$GITHUB_OUTPUT" - name: Run review checks id: review @@ -108,10 +225,20 @@ jobs: # into the script body. Raw ${{ }} interpolation of the PR title # inside run: previously allowed arbitrary shell execution in the # runner, including forcing failed=false into $GITHUB_OUTPUT. - CHANGED_FILES: ${{ steps.changed.outputs.files }} - CONTRIB_DIRS: ${{ steps.changed.outputs.contrib_dirs }} + CHANGED_FILES_JSON: ${{ steps.changed.outputs.files_json }} + CONTRIB_DIRS_JSON: ${{ steps.changed.outputs.contrib_dirs_json }} PR_TITLE: ${{ github.event.pull_request.title }} + PYTHONSAFEPATH: "1" run: | + set -euo pipefail + + jq -e 'type == "array" and all(.[]; type == "string" and (test("[\u0000-\u001F\u007F]") | not))' \ + <<< "$CHANGED_FILES_JSON" >/dev/null + jq -e 'type == "array" and all(.[]; type == "string" and test("^(recipes|schemas|dashboards|integrations|skills|primitives|extensions)/[^/]+$"))' \ + <<< "$CONTRIB_DIRS_JSON" >/dev/null + + CHANGED_FILES=$(jq -r '.[]' <<< "$CHANGED_FILES_JSON") + CONTRIB_DIRS=$(jq -r '.[]' <<< "$CONTRIB_DIRS_JSON") pass_count=0 fail_count=0 results="" @@ -220,12 +347,10 @@ jobs: echo "secret_blocked=$secret_blocked" >> "$GITHUB_OUTPUT" comment="## OB1 PR Gate\n\nThis PR does not touch contribution folders (docs/governance change). Contribution checks were skipped, but credential and binary-file scans still ran.\n\n${results}\n${summary}" - comment_delimiter="OB1_COMMENT_${GITHUB_RUN_ID}_${GITHUB_RUN_ATTEMPT}_${RANDOM}" - { - printf 'comment<<%s\n' "$comment_delimiter" - printf '%b\n' "$comment" - printf '%s\n' "$comment_delimiter" - } >> "$GITHUB_OUTPUT" + # Encode the multiline summary as one single-line output. No + # attacker-controlled GitHub multiline delimiter is used. + comment_b64=$(printf '%b\n' "$comment" | openssl base64 -A) + printf 'comment_b64=%s\n' "$comment_b64" >> "$GITHUB_OUTPUT" exit 0 fi @@ -282,7 +407,7 @@ jobs: continue fi - if ! schema_output=$(check-jsonschema --schemafile /tmp/ob1-metadata.schema.json "$dir/metadata.json" 2>&1); then + if ! schema_output=$(check-jsonschema --schemafile "$RUNNER_TEMP/ob1-metadata.schema.json" "$dir/metadata.json" 2>&1); then indented_output=$(printf '%s\n' "$schema_output" | sed 's/^/ /') rule3_detail="${rule3_detail} - \`$dir/metadata.json\` failed schema validation\n${indented_output}\n" rule3_pass=false @@ -702,22 +827,18 @@ jobs: comment="${comment}\n\n---\n\n### Post-Merge Tasks\n\n*These don't block merge — they're reminders for admins after this PR lands.*\n\n${reminders}" fi - comment_delimiter="OB1_COMMENT_${GITHUB_RUN_ID}_${GITHUB_RUN_ATTEMPT}_${RANDOM}" - { - printf 'comment<<%s\n' "$comment_delimiter" - printf '%b\n' "$comment" - printf '%s\n' "$comment_delimiter" - } >> "$GITHUB_OUTPUT" + comment_b64=$(printf '%b\n' "$comment" | openssl base64 -A) + printf 'comment_b64=%s\n' "$comment_b64" >> "$GITHUB_OUTPUT" - name: Write gate artifact env: # SECURITY: PR title, file names, and author login are untrusted; # pass them via env, never raw ${{ }} interpolation inside run:. - REVIEW_COMMENT: ${{ steps.review.outputs.comment }} + REVIEW_COMMENT_B64: ${{ steps.review.outputs.comment_b64 }} REVIEW_FAILED: ${{ steps.review.outputs.failed }} SECRET_BLOCKED: ${{ steps.review.outputs.secret_blocked }} - CHANGED_FILES: ${{ steps.changed.outputs.files }} - CONTRIB_DIRS: ${{ steps.changed.outputs.contrib_dirs }} + CHANGED_FILES_JSON: ${{ steps.changed.outputs.files_json }} + CONTRIB_DIRS_JSON: ${{ steps.changed.outputs.contrib_dirs_json }} PR_NUMBER: ${{ github.event.pull_request.number }} PR_URL: ${{ github.event.pull_request.html_url }} PR_TITLE: ${{ github.event.pull_request.title }} @@ -725,15 +846,26 @@ jobs: AUTHOR_LOGIN: ${{ github.event.pull_request.user.login }} AUTHOR_ASSOCIATION: ${{ github.event.pull_request.author_association }} IS_DRAFT: ${{ github.event.pull_request.draft }} + ARTIFACT_DIR: ${{ runner.temp }}/ob1-gate-${{ github.run_id }}-${{ github.run_attempt }} run: | set -euo pipefail - mkdir -p gate-artifact - - printf '%s\n' "$REVIEW_COMMENT" > gate-artifact/ob1-review-summary.md - printf '%s\n' "$REVIEW_COMMENT" >> "$GITHUB_STEP_SUMMARY" - printf '%s\n' "$CHANGED_FILES" > gate-artifact/changed-files.txt - printf '%s\n' "$CONTRIB_DIRS" > gate-artifact/contribution-dirs.txt + [[ "$PR_NUMBER" =~ ^[1-9][0-9]*$ ]] + [[ "$HEAD_SHA" =~ ^[0-9a-f]{40}$ ]] + [[ "$REVIEW_FAILED" =~ ^(true|false)$ ]] + [[ "$SECRET_BLOCKED" =~ ^(true|false)$ ]] + [[ "$IS_DRAFT" =~ ^(true|false)$ ]] + jq -e 'type == "array" and all(.[]; type == "string" and (test("[\u0000-\u001F\u007F]") | not))' \ + <<< "$CHANGED_FILES_JSON" >/dev/null + jq -e 'type == "array" and all(.[]; type == "string" and test("^(recipes|schemas|dashboards|integrations|skills|primitives|extensions)/[^/]+$"))' \ + <<< "$CONTRIB_DIRS_JSON" >/dev/null + + mkdir -p "$ARTIFACT_DIR" + printf '%s' "$REVIEW_COMMENT_B64" | openssl base64 -d -A > "$ARTIFACT_DIR/ob1-review-summary.md" + printf '\n' >> "$ARTIFACT_DIR/ob1-review-summary.md" + cat "$ARTIFACT_DIR/ob1-review-summary.md" >> "$GITHUB_STEP_SUMMARY" + printf '%s\n' "$CHANGED_FILES_JSON" | jq -r '.[]' > "$ARTIFACT_DIR/changed-files.txt" + printf '%s\n' "$CONTRIB_DIRS_JSON" | jq -r '.[]' > "$ARTIFACT_DIR/contribution-dirs.txt" jq -n \ --argjson pr_number "$PR_NUMBER" \ @@ -755,14 +887,14 @@ jobs: is_draft: ($is_draft == "true"), failed: ($failed == "true"), secret_blocked: ($secret_blocked == "true") - }' > gate-artifact/ob1-review-context.json + }' > "$ARTIFACT_DIR/ob1-review-context.json" - name: Upload gate artifact - if: always() uses: actions/upload-artifact@v4 with: name: ob1-pr-gate-context - path: gate-artifact/ + path: ${{ runner.temp }}/ob1-gate-${{ github.run_id }}-${{ github.run_attempt }}/ + if-no-files-found: error retention-days: 7 - name: Fail if checks failed diff --git a/.github/workflows/ob1-pr-followups.yml b/.github/workflows/ob1-pr-followups.yml index f7b92bc10..e22180dc2 100644 --- a/.github/workflows/ob1-pr-followups.yml +++ b/.github/workflows/ob1-pr-followups.yml @@ -16,18 +16,8 @@ permissions: id-token: write jobs: - ignore_non_pr_gate: - name: Ignore Non-PR Gate Run - if: github.event.workflow_run.event != 'pull_request' && github.event.workflow_run.event != 'pull_request_target' - runs-on: ubuntu-latest - steps: - - name: Explain skipped follow-up - run: | - echo "OB1 PR Follow-Ups only acts on pull_request or pull_request_target gate runs." - echo "Received upstream event: ${{ github.event.workflow_run.event }}" - followups: - if: github.event.workflow_run.event == 'pull_request' || github.event.workflow_run.event == 'pull_request_target' + if: github.event.workflow_run.event == 'pull_request' runs-on: ubuntu-latest concurrency: group: ob1-pr-followups-${{ github.event.workflow_run.pull_requests[0].number || github.event.workflow_run.id }} @@ -71,17 +61,15 @@ jobs: context_file="gate-artifact/ob1-review-context.json" summary_file="gate-artifact/ob1-review-summary.md" - { - echo "context_file=$context_file" - echo "summary_file=$summary_file" - echo "pr_number=$(jq -r '.pr_number' "$context_file")" - echo "author_login=$(jq -r '.author_login' "$context_file")" - echo "author_association=$(jq -r '.author_association' "$context_file")" - echo "head_sha=$(jq -r '.head_sha' "$context_file")" - echo "gate_failed=$(jq -r '.failed' "$context_file")" - echo "secret_blocked=$(jq -r '.secret_blocked' "$context_file")" - echo "is_draft=$(jq -r '.is_draft' "$context_file")" - } >> "$GITHUB_OUTPUT" + echo "context_file=$context_file" >> "$GITHUB_OUTPUT" + echo "summary_file=$summary_file" >> "$GITHUB_OUTPUT" + echo "pr_number=$(jq -r '.pr_number' "$context_file")" >> "$GITHUB_OUTPUT" + echo "author_login=$(jq -r '.author_login' "$context_file")" >> "$GITHUB_OUTPUT" + echo "author_association=$(jq -r '.author_association' "$context_file")" >> "$GITHUB_OUTPUT" + echo "head_sha=$(jq -r '.head_sha' "$context_file")" >> "$GITHUB_OUTPUT" + echo "gate_failed=$(jq -r '.failed' "$context_file")" >> "$GITHUB_OUTPUT" + echo "secret_blocked=$(jq -r '.secret_blocked' "$context_file")" >> "$GITHUB_OUTPUT" + echo "is_draft=$(jq -r '.is_draft' "$context_file")" >> "$GITHUB_OUTPUT" - name: Post gate summary and sync security label if: steps.artifact.outputs.found == 'true' @@ -179,11 +167,9 @@ jobs: over_quota=true fi - { - echo "trusted=$trusted" - echo "open_pr_count=$open_pr_count" - echo "over_quota=$over_quota" - } >> "$GITHUB_OUTPUT" + echo "trusted=$trusted" >> "$GITHUB_OUTPUT" + echo "open_pr_count=$open_pr_count" >> "$GITHUB_OUTPUT" + echo "over_quota=$over_quota" >> "$GITHUB_OUTPUT" - name: Sync maintainer triage label if: steps.artifact.outputs.found == 'true' From 97b866c1ae444cebb49fb17a591402839983edd7 Mon Sep 17 00:00:00 2001 From: James Hume Date: Wed, 22 Jul 2026 20:15:14 -0400 Subject: [PATCH 3/4] [docs] make gate scope comparison literal --- .github/workflows/ob1-gate-v2.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ob1-gate-v2.yml b/.github/workflows/ob1-gate-v2.yml index 0a8fab2cb..31b7c9215 100644 --- a/.github/workflows/ob1-gate-v2.yml +++ b/.github/workflows/ob1-gate-v2.yml @@ -716,7 +716,10 @@ jobs: in_scope=false while IFS= read -r dir; do [ -z "$dir" ] && continue - if echo "$f" | grep -qE "^${dir}/"; then + # Compare a literal directory prefix. Treating an untrusted + # folder name as an extended regex lets metacharacters such as + # `|` make an out-of-scope workflow file appear in scope. + if [[ "$f" == "$dir/"* ]]; then in_scope=true break fi From 027b8d2a2e264937d098248af163049311192995 Mon Sep 17 00:00:00 2001 From: James Hume Date: Wed, 22 Jul 2026 20:29:47 -0400 Subject: [PATCH 4/4] [docs] make gate file operands option-safe --- .github/workflows/ob1-gate-v2.yml | 79 ++++++++++++++++--------------- 1 file changed, 41 insertions(+), 38 deletions(-) diff --git a/.github/workflows/ob1-gate-v2.yml b/.github/workflows/ob1-gate-v2.yml index 31b7c9215..d6798e6be 100644 --- a/.github/workflows/ob1-gate-v2.yml +++ b/.github/workflows/ob1-gate-v2.yml @@ -266,11 +266,14 @@ jobs: [ ! -f "$f" ] && continue # Skip binary files to avoid noisy false positives. - if ! grep -Iq . "$f"; then + # A legal root filename can begin with a dash. Every grep file + # operand is separated from options with --, and every + # attacker-influenced pattern is passed with -e. + if ! grep -I -q -e . -- "$f"; then continue fi - matches=$(grep -nE '(sk-[A-Za-z0-9]{20,}|AKIA[0-9A-Z]{16}|AIza[0-9A-Za-z_-]{20,}|gh[pousr]_[A-Za-z0-9]{30,}|github_pat_[A-Za-z0-9_]{20,}|xox[baprs]-[A-Za-z0-9-]{10,}|SUPABASE_SERVICE_ROLE_KEY\s*=\s*"?ey)' "$f" 2>/dev/null || true) + matches=$(grep -nE -e '(sk-[A-Za-z0-9]{20,}|AKIA[0-9A-Z]{16}|AIza[0-9A-Za-z_-]{20,}|gh[pousr]_[A-Za-z0-9]{30,}|github_pat_[A-Za-z0-9_]{20,}|xox[baprs]-[A-Za-z0-9-]{10,}|SUPABASE_SERVICE_ROLE_KEY\s*=\s*"?ey)' -- "$f" 2>/dev/null || true) if [ -n "$matches" ]; then rule4_detail="${rule4_detail} - \`$f\`: potential credential found\n" rule4_pass=false @@ -278,8 +281,8 @@ jobs: fi # Check for .env files with actual values - if echo "$f" | grep -qE '\.env$'; then - has_values=$(grep -E '^[A-Z_]+=.+' "$f" 2>/dev/null | grep -vE '=(your-|<|example|placeholder|TODO)' || true) + if printf '%s\n' "$f" | grep -qE -e '\.env$'; then + has_values=$(grep -E -e '^[A-Z_]+=.+' -- "$f" 2>/dev/null | grep -vE -e '=(your-|<|example|placeholder|TODO)' || true) if [ -n "$has_values" ]; then rule4_detail="${rule4_detail} - \`$f\`: .env file appears to contain real values\n" rule4_pass=false @@ -312,7 +315,7 @@ jobs: fi # Check banned extensions - if echo "$f" | grep -qE '\.(exe|dmg|zip|tar\.gz|tar\.bz2|rar|7z|msi|pkg|deb|rpm)$'; then + if printf '%s\n' "$f" | grep -qE -e '\.(exe|dmg|zip|tar\.gz|tar\.bz2|rar|7z|msi|pkg|deb|rpm)$'; then rule8_detail="${rule8_detail} - \`$f\`: binary/archive files not allowed\n" rule8_pass=false fi @@ -360,7 +363,7 @@ jobs: allowed_pattern='^(recipes|schemas|dashboards|integrations|skills|primitives|extensions|docs|resources|\.github|\.gitkeep|\.gitignore|\.markdownlint)' while IFS= read -r f; do [ -z "$f" ] && continue - if ! echo "$f" | grep -qE "$allowed_pattern"; then + if ! printf '%s\n' "$f" | grep -qE -e "$allowed_pattern"; then bad_files="${bad_files} - \`$f\`\n" rule1_pass=false fi @@ -432,25 +435,25 @@ jobs: case "$f" in *.sql) ;; *) continue ;; esac # Check for destructive operations - dangerous=$(grep -niE '(DROP\s+TABLE|DROP\s+DATABASE|TRUNCATE)' "$f" 2>/dev/null || true) + dangerous=$(grep -niE -e '(DROP\s+TABLE|DROP\s+DATABASE|TRUNCATE)' -- "$f" 2>/dev/null || true) if [ -n "$dangerous" ]; then rule5_detail="${rule5_detail} - \`$f\`: contains DROP/TRUNCATE statement\n" rule5_pass=false fi # Check for DELETE without WHERE - deletes=$(grep -niE 'DELETE\s+FROM' "$f" 2>/dev/null || true) + deletes=$(grep -niE -e 'DELETE\s+FROM' -- "$f" 2>/dev/null || true) if [ -n "$deletes" ]; then while IFS= read -r line; do - if ! echo "$line" | grep -qiE 'WHERE'; then - rule5_detail="${rule5_detail} - \`$f\` line $(echo "$line" | cut -d: -f1): DELETE FROM without WHERE clause\n" + if ! printf '%s\n' "$line" | grep -qiE -e 'WHERE'; then + rule5_detail="${rule5_detail} - \`$f\` line $(printf '%s\n' "$line" | cut -d: -f1): DELETE FROM without WHERE clause\n" rule5_pass=false fi done <<< "$deletes" fi # Check for altering/dropping core thoughts table columns - alter_thoughts=$(grep -niE 'ALTER\s+TABLE\s+thoughts\s+(DROP|ALTER)\s+COLUMN' "$f" 2>/dev/null || true) + alter_thoughts=$(grep -niE -e 'ALTER\s+TABLE\s+thoughts\s+(DROP|ALTER)\s+COLUMN' -- "$f" 2>/dev/null || true) if [ -n "$alter_thoughts" ]; then rule5_detail="${rule5_detail} - \`$f\`: modifies core thoughts table columns (only ADD COLUMN is allowed)\n" rule5_pass=false @@ -468,16 +471,16 @@ jobs: rule6_detail="" while IFS= read -r dir; do [ -z "$dir" ] && continue - category=$(echo "$dir" | cut -d'/' -f1) - dir_files=$(find "$dir" -type f 2>/dev/null | grep -v README.md | grep -v metadata.json || true) + category=$(printf '%s\n' "$dir" | cut -d'/' -f1) + dir_files=$(find "$dir" -type f 2>/dev/null | grep -v -e README.md | grep -v -e metadata.json || true) case "$category" in recipes) - has_artifact=$(echo "$dir_files" | grep -E '\.(sql|ts|js|py)$' || true) + has_artifact=$(printf '%s\n' "$dir_files" | grep -E -e '\.(sql|ts|js|py)$' || true) # grep -c prints "0" itself on no match (and exits 1), so an # "|| echo 0" fallback produced "0\n0", which broke the -lt # comparison below and let empty recipes pass silently. - readme_has_steps=$(grep -c -iE '^\s*[0-9]+\.' "$dir/README.md" 2>/dev/null || true) + readme_has_steps=$(grep -c -iE -e '^\s*[0-9]+\.' -- "$dir/README.md" 2>/dev/null || true) readme_has_steps=${readme_has_steps:-0} if [ -z "$has_artifact" ] && [ "$readme_has_steps" -lt 3 ]; then rule6_detail="${rule6_detail} - \`$dir\`: recipes need code files (.sql/.ts/.js/.py) or detailed step-by-step instructions in README\n" @@ -485,14 +488,14 @@ jobs: fi ;; schemas) - has_sql=$(echo "$dir_files" | grep -E '\.sql$' || true) + has_sql=$(printf '%s\n' "$dir_files" | grep -E -e '\.sql$' || true) if [ -z "$has_sql" ]; then rule6_detail="${rule6_detail} - \`$dir\`: schemas must contain at least one .sql file\n" rule6_pass=false fi ;; dashboards) - has_frontend=$(echo "$dir_files" | grep -E '\.(html|jsx|tsx|vue|svelte)$' || true) + has_frontend=$(printf '%s\n' "$dir_files" | grep -E -e '\.(html|jsx|tsx|vue|svelte)$' || true) has_pkg=$(find "$dir" -name "package.json" 2>/dev/null || true) if [ -z "$has_frontend" ] && [ -z "$has_pkg" ]; then rule6_detail="${rule6_detail} - \`$dir\`: dashboards must contain frontend code (.html/.jsx/.tsx/.vue/.svelte) or package.json\n" @@ -500,14 +503,14 @@ jobs: fi ;; integrations) - has_code=$(echo "$dir_files" | grep -E '\.(ts|js|py)$' || true) + has_code=$(printf '%s\n' "$dir_files" | grep -E -e '\.(ts|js|py)$' || true) if [ -z "$has_code" ]; then rule6_detail="${rule6_detail} - \`$dir\`: integrations must contain code files (.ts/.js/.py)\n" rule6_pass=false fi ;; skills) - has_skill=$(echo "$dir_files" | grep -iE '(^|/)(SKILL\.md|[^/]+[.-]skill\.md)$' || true) + has_skill=$(printf '%s\n' "$dir_files" | grep -iE -e '(^|/)(SKILL\.md|[^/]+[.-]skill\.md)$' || true) if [ -z "$has_skill" ]; then rule6_detail="${rule6_detail} - \`$dir\`: skills must contain a plain-text skill file (\`SKILL.md\`, \`*.skill.md\`, or \`*-skill.md\`)\n" rule6_pass=false @@ -525,8 +528,8 @@ jobs: ;; extensions) # Extensions must have both SQL and code files - has_sql=$(echo "$dir_files" | grep -E '\.sql$' || true) - has_code=$(echo "$dir_files" | grep -E '\.(ts|js|py)$' || true) + has_sql=$(printf '%s\n' "$dir_files" | grep -E -e '\.sql$' || true) + has_code=$(printf '%s\n' "$dir_files" | grep -E -e '\.(ts|js|py)$' || true) if [ -z "$has_sql" ]; then rule6_detail="${rule6_detail} - \`$dir\`: extensions must contain at least one .sql file\n" rule6_pass=false @@ -546,7 +549,7 @@ jobs: fi # ─── Rule 7: PR format ─── - if echo "$PR_TITLE" | grep -qE '^\[(recipes|schemas|dashboards|integrations|skills|primitives|extensions|docs)\] '; then + if printf '%s\n' "$PR_TITLE" | grep -qE -e '^\[(recipes|schemas|dashboards|integrations|skills|primitives|extensions|docs)\] '; then pass_check "PR format" "Title follows \`[category] Description\` format" else fail_check "PR format" "PR title must start with \`[recipes]\`, \`[schemas]\`, \`[dashboards]\`, \`[integrations]\`, \`[skills]\`, \`[primitives]\`, \`[extensions]\`, or \`[docs]\` followed by a space and description" @@ -565,13 +568,13 @@ jobs: readme="$dir/README.md" missing_sections="" - if ! grep -qi 'prerequisite' "$readme"; then + if ! grep -qi -e 'prerequisite' -- "$readme"; then missing_sections="${missing_sections}Prerequisites, " fi - if ! grep -qiE '^\s*[0-9]+\.' "$readme"; then + if ! grep -qiE -e '^\s*[0-9]+\.' -- "$readme"; then missing_sections="${missing_sections}Step-by-step instructions, " fi - if ! grep -qiE '(expected|outcome|result)' "$readme"; then + if ! grep -qiE -e '(expected|outcome|result)' -- "$readme"; then missing_sections="${missing_sections}Expected outcome, " fi @@ -609,7 +612,7 @@ jobs: # (b) Check that the README links to the primitive if [ -f "$readme" ]; then - if ! grep -q "primitives/$prim" "$readme"; then + if ! grep -Fq -e "primitives/$prim" -- "$readme"; then rule10_detail="${rule10_detail} - \`$dir/README.md\`: declares dependency on primitive \`$prim\` but does not link to it\n" rule10_pass=false fi @@ -629,7 +632,7 @@ jobs: # (b) Check that the README links to the skill if [ -f "$readme" ]; then - if ! grep -q "skills/$skill" "$readme"; then + if ! grep -Fq -e "skills/$skill" -- "$readme"; then rule10_detail="${rule10_detail} - \`$dir/README.md\`: declares dependency on skill \`$skill\` but does not link to it\n" rule10_pass=false fi @@ -661,10 +664,10 @@ jobs: case "$f" in *.md|*.ts|*.js) ;; *) continue ;; esac # Check for old local MCP config pattern - local_mcp=$(grep -niE '(claude_desktop_config|"command":\s*"node"|StdioServerTransport|mcpServers.*command)' "$f" 2>/dev/null || true) + local_mcp=$(grep -niE -e '(claude_desktop_config|"command":\s*"node"|StdioServerTransport|mcpServers.*command)' -- "$f" 2>/dev/null || true) if [ -n "$local_mcp" ]; then # Skip _template files (they might reference the pattern to warn against it) - if echo "$f" | grep -q '_template'; then + if printf '%s\n' "$f" | grep -q -e '_template'; then continue fi rule14_detail="${rule14_detail} - \`$f\`: references local MCP pattern (claude_desktop_config.json or stdio transport). Extensions must use remote MCP via Supabase Edge Functions.\n" @@ -685,7 +688,7 @@ jobs: rule15_detail="" while IFS= read -r dir; do [ -z "$dir" ] && continue - category=$(echo "$dir" | cut -d'/' -f1) + category=$(printf '%s\n' "$dir" | cut -d'/' -f1) case "$category" in extensions|integrations) ;; *) continue ;; @@ -693,7 +696,7 @@ jobs: [ ! -f "$dir/README.md" ] && continue # Check for any link to the tool audit guide - if ! grep -q '05-tool-audit' "$dir/README.md"; then + if ! grep -q -e '05-tool-audit' -- "$dir/README.md"; then rule15_detail="${rule15_detail} - \`$dir/README.md\`: missing link to [MCP Tool Audit & Optimization Guide](docs/05-tool-audit.md). Required for extensions and integrations that expose MCP tools.\n" rule15_pass=false fi @@ -745,7 +748,7 @@ jobs: [ ! -f "$dir/README.md" ] && continue # Extract relative markdown links (not http, not anchors) - links=$(grep -oE '\]\([^)]+\)' "$dir/README.md" | sed 's/^\](//' | sed 's/)$//' | grep -v '^http' | grep -v '^#' || true) + links=$(grep -oE -e '\]\([^)]+\)' -- "$dir/README.md" | sed 's/^\](//' | sed 's/)$//' | grep -v -e '^http' | grep -v -e '^#' || true) while IFS= read -r link; do [ -z "$link" ] && continue @@ -772,8 +775,8 @@ jobs: reminders="" while IFS= read -r dir; do [ -z "$dir" ] && continue - category=$(echo "$dir" | cut -d'/' -f1) - folder_name=$(echo "$dir" | cut -d'/' -f2) + category=$(printf '%s\n' "$dir" | cut -d'/' -f1) + folder_name=$(printf '%s\n' "$dir" | cut -d'/' -f2) contrib_name=$(jq -r '.name // empty' "$dir/metadata.json" 2>/dev/null || echo "$folder_name") author_name=$(jq -r '.author.name // empty' "$dir/metadata.json" 2>/dev/null || echo "unknown") author_github=$(jq -r '.author.github // empty' "$dir/metadata.json" 2>/dev/null || echo "") @@ -781,14 +784,14 @@ jobs: # Check category index file index_file="$category/README.md" if [ -f "$index_file" ]; then - if ! grep -q "$folder_name" "$index_file"; then + if ! grep -Fq -e "$folder_name" -- "$index_file"; then reminders="${reminders}- [ ] Add **${contrib_name}** to [\`${index_file}\`](${index_file})\n" fi fi # Check root README community section if [ -f "README.md" ]; then - if ! grep -qi "$folder_name" "README.md"; then + if ! grep -Fqi -e "$folder_name" -- "README.md"; then reminders="${reminders}- [ ] Add **${contrib_name}** to root README.md community contributions section\n" fi fi @@ -796,10 +799,10 @@ jobs: # Check CONTRIBUTORS.md if [ -f "CONTRIBUTORS.md" ]; then found_contributor=false - if [ -n "$author_name" ] && grep -qi "$author_name" "CONTRIBUTORS.md"; then + if [ -n "$author_name" ] && grep -Fqi -e "$author_name" -- "CONTRIBUTORS.md"; then found_contributor=true fi - if [ -n "$author_github" ] && grep -qi "$author_github" "CONTRIBUTORS.md"; then + if [ -n "$author_github" ] && grep -Fqi -e "$author_github" -- "CONTRIBUTORS.md"; then found_contributor=true fi if [ "$found_contributor" = false ]; then