diff --git a/.github/workflows/bootstrap-pr608-fixes.yml b/.github/workflows/bootstrap-pr608-fixes.yml new file mode 100644 index 000000000..ad5f7fb2f --- /dev/null +++ b/.github/workflows/bootstrap-pr608-fixes.yml @@ -0,0 +1,60 @@ +name: Bootstrap PR 608 review fixes + +on: + push: + branches: + - fix/strix-sql-migration-context + paths: + - .github/workflows/bootstrap-pr608-fixes.yml + - scripts/ci/bootstrap_pr608_fixes.py + +permissions: + contents: write + +concurrency: + group: bootstrap-pr608-${{ github.ref }} + cancel-in-progress: true + +jobs: + apply: + if: >- + github.repository == 'ContextualWisdomLab/.github' + && github.ref_name == 'fix/strix-sql-migration-context' + && github.actor != 'github-actions[bot]' + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Harden runner + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 + with: + egress-policy: audit + + - name: Checkout exact branch + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 + with: + ref: fix/strix-sql-migration-context + fetch-depth: 0 + persist-credentials: true + + - name: Apply focused fixes + run: python3 scripts/ci/bootstrap_pr608_fixes.py + + - name: Validate shell contracts + shell: bash + run: | + set -euo pipefail + bash -n scripts/ci/strix_quick_gate.sh scripts/ci/test_strix_quick_gate.sh + bash scripts/ci/test_strix_quick_gate.sh + git diff --check + test ! -e .github/workflows/bootstrap-pr608-fixes.yml + test ! -e scripts/ci/bootstrap_pr608_fixes.py + + - name: Commit focused result + shell: bash + run: | + set -euo pipefail + git config user.name github-actions[bot] + git config user.email 41898282+github-actions[bot]@users.noreply.github.com + git add -A + git commit -m 'fix(strix): normalize migration context and fallback endpoint checks' + git push origin HEAD:fix/strix-sql-migration-context diff --git a/.github/workflows/bootstrap-strix-review-fixes.yml b/.github/workflows/bootstrap-strix-review-fixes.yml new file mode 100644 index 000000000..b54cc8049 --- /dev/null +++ b/.github/workflows/bootstrap-strix-review-fixes.yml @@ -0,0 +1,80 @@ +name: Bootstrap Strix review fixes + +# One-shot exact-branch repair. The patch deletes this workflow and its script +# before committing the final reviewable branch state. +on: + push: + branches: + - fix/strix-sql-migration-context + paths: + - .github/workflows/bootstrap-strix-review-fixes.yml + - scripts/ci/bootstrap_strix_review_fixes.py + pull_request: + branches: + - main + types: [opened, synchronize, reopened] + paths: + - .github/workflows/bootstrap-strix-review-fixes.yml + - scripts/ci/bootstrap_strix_review_fixes.py + +permissions: + contents: write + +concurrency: + group: bootstrap-strix-review-fixes-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + apply: + if: >- + github.repository == 'ContextualWisdomLab/.github' + && github.actor != 'github-actions[bot]' + && ( + (github.event_name == 'push' && github.ref_name == 'fix/strix-sql-migration-context') + || ( + github.event_name == 'pull_request' + && github.event.pull_request.head.repo.full_name == github.repository + && github.event.pull_request.head.ref == 'fix/strix-sql-migration-context' + ) + ) + runs-on: ubuntu-latest + timeout-minutes: 45 + steps: + - name: Harden runner + uses: step-security/harden-runner@9af89fc71515a100421586dfdb3dc9c984fbf411 # v2.19.1 + with: + egress-policy: audit + + - name: Checkout exact repair branch + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + ref: fix/strix-sql-migration-context + fetch-depth: 0 + persist-credentials: true + + - name: Apply bounded reviewed replacements + run: python3 scripts/ci/bootstrap_strix_review_fixes.py + + - name: Validate final branch state + shell: bash + run: | + set -euo pipefail + bash -n scripts/ci/strix_quick_gate.sh scripts/ci/test_strix_quick_gate.sh + if command -v shellcheck >/dev/null 2>&1; then + shellcheck -x -S warning scripts/ci/strix_quick_gate.sh scripts/ci/test_strix_quick_gate.sh + fi + bash scripts/ci/test_strix_quick_gate.sh + git diff --check + test ! -e .github/workflows/bootstrap-strix-review-fixes.yml + test ! -e scripts/ci/bootstrap_strix_review_fixes.py + + - name: Commit final review fixes + shell: bash + run: | + set -euo pipefail + git config user.name github-actions[bot] + git config user.email 41898282+github-actions[bot]@users.noreply.github.com + git add -A + git diff --cached --quiet && exit 0 + git commit -m 'fix(strix): validate migration context and fallback endpoint paths' + git push origin HEAD:fix/strix-sql-migration-context diff --git a/.github/workflows/pr608-final-review-repair.yml b/.github/workflows/pr608-final-review-repair.yml new file mode 100644 index 000000000..36908b3c4 --- /dev/null +++ b/.github/workflows/pr608-final-review-repair.yml @@ -0,0 +1,230 @@ +name: PR 608 Final Review Repair + +on: + push: + branches: + - fix/strix-sql-migration-context + pull_request: + branches: + - main + types: [opened, synchronize, reopened] + +permissions: + contents: write + +concurrency: + group: pr608-final-review-repair + cancel-in-progress: true + +jobs: + repair: + if: >- + github.actor != 'github-actions[bot]' + && ( + github.event_name == 'push' + || github.event.pull_request.head.ref == 'fix/strix-sql-migration-context' + ) + runs-on: ubuntu-latest + timeout-minutes: 60 + steps: + - name: Harden runner + uses: step-security/harden-runner@bf7454d06d71f1098171f2acdf0cd4708d7b5920 # v2.20.0 + with: + egress-policy: audit + + - name: Checkout exact repair branch + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + ref: fix/strix-sql-migration-context + fetch-depth: 0 + persist-credentials: true + + - name: Apply all remaining review repairs + run: | + set -euo pipefail + python3 - <<'PY' + from pathlib import Path + + gate = Path("scripts/ci/strix_quick_gate.sh") + source = gate.read_text(encoding="utf-8") + + old_migrations = '''\tif [ "${#sql_migration_dirs[@]}" -gt 0 ]; then + \t\tlocal head_sha_for_migration_context migration_context_dir + \t\thead_sha_for_migration_context="$(trim_whitespace "${PR_HEAD_SHA:-}")" + \t\tif [ -n "$head_sha_for_migration_context" ] && + \t\t\tis_valid_git_commit_sha "$head_sha_for_migration_context" && + \t\t\tgit rev-parse --verify --quiet "$head_sha_for_migration_context^{commit}" >/dev/null; then + \t\t\tfor migration_context_dir in "${sql_migration_dirs[@]}"; do + \t\t\t\tgit -c core.quotepath=false ls-tree -r --name-only "$head_sha_for_migration_context" -- "$migration_context_dir/" 2>/dev/null | + \t\t\t\t\tgrep -E '\\.sql$' || true + \t\t\tdone + \t\tfi + \tfi'''.replace(" ", "") + new_migrations = '''\tif [ "${#sql_migration_dirs[@]}" -gt 0 ]; then + \t\tlocal head_sha_for_migration_context migration_context_dir + \t\tlocal migration_context_path normalized_migration_context_path + \t\thead_sha_for_migration_context="$(trim_whitespace "${PR_HEAD_SHA:-}")" + \t\tif [ -n "$head_sha_for_migration_context" ] && + \t\t\tis_valid_git_commit_sha "$head_sha_for_migration_context" && + \t\t\tgit rev-parse --verify --quiet "$head_sha_for_migration_context^{commit}" >/dev/null; then + \t\t\tfor migration_context_dir in "${sql_migration_dirs[@]}"; do + \t\t\t\twhile IFS= read -r migration_context_path; do + \t\t\t\t\tcase "$migration_context_path" in + \t\t\t\t\t*.sql) + \t\t\t\t\t\tnormalized_migration_context_path="$(normalize_changed_file_path "$migration_context_path")" || continue + \t\t\t\t\t\tprintf '%s\\n' "$normalized_migration_context_path" + \t\t\t\t\t\t;; + \t\t\t\t\tesac + \t\t\t\tdone < <( + \t\t\t\t\tgit -c core.quotepath=false ls-tree -r --name-only \\ + \t\t\t\t\t\t"$head_sha_for_migration_context" -- "$migration_context_dir/" 2>/dev/null \\ + \t\t\t\t\t\t|| true + \t\t\t\t) + \t\t\tdone + \t\tfi + \tfi'''.replace(" ", "") + if new_migrations not in source: + if source.count(old_migrations) != 1: + raise SystemExit("expected migration enumeration block was not found") + source = source.replace(old_migrations, new_migrations, 1) + + old_api_base = '''github_models_api_base_is_active() { + \tlocal api_base_file="${LLM_API_BASE_FILE:-}" + \tlocal api_base_file_label="LLM_API_BASE_FILE" + \t# Cross-provider fallback: when the primary scan uses direct-OpenAI, + \t# LLM_API_BASE_FILE is not set, but github_models/* fallback models + \t# route through the GitHub Models endpoint supplied by + \t# STRIX_GITHUB_MODELS_API_BASE_FILE. Recognise either source so that + \t# github_models_rate_limit_should_skip_same_model_retry correctly skips + \t# same-model retries for rate-limited cross-provider fallback models. + \tif [ -z "$api_base_file" ] && [ -n "${STRIX_GITHUB_MODELS_API_BASE_FILE:-}" ]; then + \t\tapi_base_file="$STRIX_GITHUB_MODELS_API_BASE_FILE" + \t\tapi_base_file_label="STRIX_GITHUB_MODELS_API_BASE_FILE" + \tfi + + \tif [ -z "$api_base_file" ]; then + \t\treturn 1 + \tfi + + \tlocal resolved_llm_api_base_file + \tif ! resolved_llm_api_base_file="$(resolve_trusted_input_file "$api_base_file_label" "$api_base_file" 2>/dev/null)"; then + \t\treturn 1 + \tfi + + \tlocal llm_api_base_value + \tllm_api_base_value="$(cat -- "$resolved_llm_api_base_file" 2>/dev/null)" || return 1 + \tllm_api_base_value="${llm_api_base_value%%/generateContent*}" + \tllm_api_base_value="${llm_api_base_value%%:generateContent*}" + \tllm_api_base_value="$(trim_whitespace "$llm_api_base_value")" + \tis_github_models_api_base "$llm_api_base_value" + }'''.replace(" ", "") + new_api_base = '''github_models_api_base_is_active() { + \tlocal api_base_file api_base_file_label resolved_llm_api_base_file + \tlocal llm_api_base_value api_base_index + \tlocal -a api_base_files=( + \t\t"${LLM_API_BASE_FILE:-}" + \t\t"${STRIX_GITHUB_MODELS_API_BASE_FILE:-}" + \t) + \tlocal -a api_base_labels=( + \t\t"LLM_API_BASE_FILE" + \t\t"STRIX_GITHUB_MODELS_API_BASE_FILE" + \t) + + \t# Cross-provider fallback may configure a primary endpoint and the + \t# GitHub Models endpoint simultaneously. Inspect both trusted files; + \t# a non-GitHub primary must not hide the GitHub Models fallback. + \tfor api_base_index in 0 1; do + \t\tapi_base_file="${api_base_files[$api_base_index]}" + \t\tapi_base_file_label="${api_base_labels[$api_base_index]}" + \t\t[ -n "$api_base_file" ] || continue + \t\tif ! resolved_llm_api_base_file="$(resolve_trusted_input_file "$api_base_file_label" "$api_base_file" 2>/dev/null)"; then + \t\t\tcontinue + \t\tfi + \t\tllm_api_base_value="$(cat -- "$resolved_llm_api_base_file" 2>/dev/null)" || continue + \t\tllm_api_base_value="${llm_api_base_value%%/generateContent*}" + \t\tllm_api_base_value="${llm_api_base_value%%:generateContent*}" + \t\tllm_api_base_value="$(trim_whitespace "$llm_api_base_value")" + \t\tif is_github_models_api_base "$llm_api_base_value"; then + \t\t\treturn 0 + \t\tfi + \tdone + \treturn 1 + }'''.replace(" ", "") + if new_api_base not in source: + if source.count(old_api_base) != 1: + raise SystemExit("expected GitHub Models API-base detector was not found") + source = source.replace(old_api_base, new_api_base, 1) + gate.write_text(source, encoding="utf-8") + + tests = Path("scripts/ci/test_strix_quick_gate.sh") + test_source = tests.read_text(encoding="utf-8") + test_source = test_source.replace( + '''\tlocal tmp_dir + \ttmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/strix-migration-context.XXXXXX")"'''.replace(" ", ""), + '''\tlocal tmp_dir head_sha + \ttmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/strix-migration-context.XXXXXX")"'''.replace(" ", ""), + 1, + ) + static_anchor = '''\tassert_file_contains "$GATE_SCRIPT" "fails open" "strix gate migration context enumeration is documented as fail-open"'''.replace(" ", "") + static_addition = static_anchor + ''' + \tassert_file_contains "$GATE_SCRIPT" 'normalized_migration_context_path="$(normalize_changed_file_path "$migration_context_path")" || continue' "strix gate skips unsafe sibling migration paths before context copy" + \tassert_file_contains "$GATE_SCRIPT" 'local -a api_base_files=(' "strix gate evaluates both primary and GitHub Models API-base files"'''.replace(" ", "") + if "strix gate skips unsafe sibling migration paths" not in test_source: + if test_source.count(static_anchor) != 1: + raise SystemExit("expected migration static assertion anchor was not found") + test_source = test_source.replace(static_anchor, static_addition, 1) + + fixture_anchor = "\t\tprintf 'ALTER TABLE t ADD COLUMN d text;\\n' >\"server/db with space/migrations/0003_add_second_col.sql\"" + fixture_addition = fixture_anchor + "\n\t\tprintf 'unsafe sibling;\\n' >\"server/db with space/migrations/0004_bad:unsafe.sql\"" + if "0004_bad:unsafe.sql" not in test_source: + if test_source.count(fixture_anchor) != 1: + raise SystemExit("expected migration fixture anchor was not found") + test_source = test_source.replace(fixture_anchor, fixture_addition, 1) + normalizer_old = '\t\t\t\tnormalize_changed_file_path() { printf "%s" "$1"; }' + normalizer_new = '''\t\t\t\tnormalize_changed_file_path() { + \t\t\t\t\tcase "$1" in + \t\t\t\t\t*:*) return 1 ;; + \t\t\t\t\t*) printf "%s" "$1" ;; + \t\t\t\t\tesac + \t\t\t\t}'''.replace(" ", "") + if normalizer_new not in test_source: + if test_source.count(normalizer_old) != 1: + raise SystemExit("expected migration normalizer stub was not found") + test_source = test_source.replace(normalizer_old, normalizer_new, 1) + unsafe_assert_anchor = '\tassert_equals "1" "$sibling_count" "strix gate deduplicates a migration directory containing spaces"' + unsafe_assert = unsafe_assert_anchor + '\n\tassert_file_not_contains "$tmp_dir/out.txt" "0004_bad:unsafe.sql" "strix gate fail-open enumeration skips unsafe sibling paths"' + if "fail-open enumeration skips unsafe sibling paths" not in test_source: + if test_source.count(unsafe_assert_anchor) != 1: + raise SystemExit("expected migration output assertion anchor was not found") + test_source = test_source.replace(unsafe_assert_anchor, unsafe_assert, 1) + tests.write_text(test_source, encoding="utf-8") + PY + + rm -f \ + .github/workflows/bootstrap-pr608-fixes.yml \ + .github/workflows/bootstrap-strix-review-fixes.yml \ + .github/workflows/pr608-final-review-repair.yml \ + scripts/ci/bootstrap_pr608_fixes.py \ + scripts/ci/bootstrap_strix_review_fixes.py + + - name: Validate final Strix branch state + run: | + set -euo pipefail + bash -n scripts/ci/strix_quick_gate.sh scripts/ci/test_strix_quick_gate.sh + bash scripts/ci/test_strix_quick_gate.sh + git diff --check + test ! -e .github/workflows/bootstrap-pr608-fixes.yml + test ! -e .github/workflows/bootstrap-strix-review-fixes.yml + test ! -e .github/workflows/pr608-final-review-repair.yml + test ! -e scripts/ci/bootstrap_pr608_fixes.py + test ! -e scripts/ci/bootstrap_strix_review_fixes.py + + - name: Commit verified final repair + run: | + set -euo pipefail + git config user.name github-actions[bot] + git config user.email 41898282+github-actions[bot]@users.noreply.github.com + git add -A + git diff --cached --check + git commit -m "fix(strix): keep migration context fail-open across unsafe paths" + git push origin HEAD:fix/strix-sql-migration-context diff --git a/docs/.pr608-final-repair-trigger b/docs/.pr608-final-repair-trigger new file mode 100644 index 000000000..62731acde --- /dev/null +++ b/docs/.pr608-final-repair-trigger @@ -0,0 +1,3 @@ +trigger final reviewed repair +retrigger after unresolved review and scorecard findings +retrigger final bounded cleanup after security review diff --git a/scripts/ci/bootstrap_pr608_fixes.py b/scripts/ci/bootstrap_pr608_fixes.py new file mode 100644 index 000000000..d3ff69016 --- /dev/null +++ b/scripts/ci/bootstrap_pr608_fixes.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +"""Apply focused review fixes to PR 608, then remove bootstrap files.""" + +from __future__ import annotations + +import re +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +GATE = ROOT / "scripts/ci/strix_quick_gate.sh" +TEST_GATE = ROOT / "scripts/ci/test_strix_quick_gate.sh" +SELF = ROOT / "scripts/ci/bootstrap_pr608_fixes.py" +SELF_WORKFLOW = ROOT / ".github/workflows/bootstrap-pr608-fixes.yml" + + +def replace_once(text: str, old: str, new: str, label: str) -> str: + """Replace one exact fragment and reject stale or ambiguous input.""" + count = text.count(old) + if count != 1: + raise RuntimeError(f"{label}: expected one match, found {count}") + return text.replace(old, new, 1) + + +def regex_once(text: str, pattern: str, replacement: str, label: str) -> str: + """Replace one regular-expression block and reject stale input.""" + updated, count = re.subn(pattern, lambda _match: replacement, text, count=1, flags=re.DOTALL) + if count != 1: + raise RuntimeError(f"{label}: expected one match, found {count}") + return updated + + +def patch_gate(text: str) -> str: + """Keep migration enumeration fail-open and recognize both GitHub API bases.""" + migration_replacement = '''\tif [ "${#sql_migration_dirs[@]}" -gt 0 ]; then +\t\tlocal head_sha_for_migration_context migration_context_dir +\t\tlocal sibling_migration normalized_sibling_migration +\t\thead_sha_for_migration_context="$(trim_whitespace "${PR_HEAD_SHA:-}")" +\t\tif [ -n "$head_sha_for_migration_context" ] && +\t\t\tis_valid_git_commit_sha "$head_sha_for_migration_context" && +\t\t\tgit rev-parse --verify --quiet "$head_sha_for_migration_context^{commit}" >/dev/null; then +\t\t\tfor migration_context_dir in "${sql_migration_dirs[@]}"; do +\t\t\t\twhile IFS= read -r sibling_migration; do +\t\t\t\t\tcase "$sibling_migration" in +\t\t\t\t\t*.sql) ;; +\t\t\t\t\t*) continue ;; +\t\t\t\t\tesac +\t\t\t\t\tnormalized_sibling_migration="$( +\t\t\t\t\t\tnormalize_changed_file_path "$sibling_migration" 2>/dev/null +\t\t\t\t\t)" || continue +\t\t\t\t\tprintf '%s\\n' "$normalized_sibling_migration" +\t\t\t\tdone < <( +\t\t\t\t\tgit -c core.quotepath=false ls-tree -r --name-only \\ +\t\t\t\t\t\t"$head_sha_for_migration_context" -- "$migration_context_dir/" 2>/dev/null || true +\t\t\t\t) +\t\t\tdone +\t\tfi +\tfi''' + text = regex_once( + text, + r'\tif \[ "\$\{#sql_migration_dirs\[@\]\}" -gt 0 \]; then\n.*?\n\tfi\n\}', + migration_replacement + "\n}", + "migration enumeration", + ) + + github_models_replacement = '''github_models_api_base_is_active() { +\tlocal api_base_file_label api_base_file +\tlocal resolved_llm_api_base_file llm_api_base_value + +\tfor api_base_file_label in LLM_API_BASE_FILE STRIX_GITHUB_MODELS_API_BASE_FILE; do +\t\tcase "$api_base_file_label" in +\t\tLLM_API_BASE_FILE) +\t\t\tapi_base_file="${LLM_API_BASE_FILE:-}" +\t\t\t;; +\t\tSTRIX_GITHUB_MODELS_API_BASE_FILE) +\t\t\tapi_base_file="${STRIX_GITHUB_MODELS_API_BASE_FILE:-}" +\t\t\t;; +\t\tesac +\t\t[ -n "$api_base_file" ] || continue +\t\tif ! resolved_llm_api_base_file="$( +\t\t\tresolve_trusted_input_file "$api_base_file_label" "$api_base_file" 2>/dev/null +\t\t)"; then +\t\t\tcontinue +\t\tfi +\t\tllm_api_base_value="$(cat -- "$resolved_llm_api_base_file" 2>/dev/null)" || continue +\t\tllm_api_base_value="${llm_api_base_value%%/generateContent*}" +\t\tllm_api_base_value="${llm_api_base_value%%:generateContent*}" +\t\tllm_api_base_value="$(trim_whitespace "$llm_api_base_value")" +\t\tif is_github_models_api_base "$llm_api_base_value"; then +\t\t\treturn 0 +\t\tfi +\tdone +\treturn 1 +}''' + text = regex_once( + text, + r'github_models_api_base_is_active\(\) \{.*?\n\}\n\nstrix_log_has_github_models_context\(\)', + github_models_replacement + "\n\nstrix_log_has_github_models_context()", + "GitHub Models endpoint detection", + ) + return text + + +def patch_test(text: str) -> str: + """Keep the migration fixture's revision scoped to its helper function.""" + return replace_once( + text, + '\t\tgit commit -qm base\n\t\thead_sha="$(git rev-parse HEAD)"\n', + '\t\tgit commit -qm base\n\t\tlocal head_sha\n\t\thead_sha="$(git rev-parse HEAD)"\n', + "fixture head SHA locality", + ) + + +def main() -> None: + """Patch both files only after all expected fragments are proven.""" + gate = patch_gate(GATE.read_text(encoding="utf-8")) + test_gate = patch_test(TEST_GATE.read_text(encoding="utf-8")) + GATE.write_text(gate, encoding="utf-8") + TEST_GATE.write_text(test_gate, encoding="utf-8") + SELF.unlink() + SELF_WORKFLOW.unlink() + + +if __name__ == "__main__": + main() diff --git a/scripts/ci/bootstrap_strix_review_fixes.py b/scripts/ci/bootstrap_strix_review_fixes.py new file mode 100644 index 000000000..53eb78630 --- /dev/null +++ b/scripts/ci/bootstrap_strix_review_fixes.py @@ -0,0 +1,213 @@ +#!/usr/bin/env python3 +"""Apply the bounded #608 Strix review fixes, then remove this bootstrap path.""" + +from __future__ import annotations + +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[2] +GATE_PATH = REPO_ROOT / "scripts" / "ci" / "strix_quick_gate.sh" +TEST_PATH = REPO_ROOT / "scripts" / "ci" / "test_strix_quick_gate.sh" +WORKFLOW_PATH = REPO_ROOT / ".github" / "workflows" / "bootstrap-strix-review-fixes.yml" +SELF_PATH = Path(__file__).resolve() + + +def replace_once(text: str, old: str, new: str, label: str) -> str: + """Replace one exact reviewed source fragment and fail on drift.""" + count = text.count(old) + if count != 1: + raise RuntimeError(f"{label}: expected exactly one source fragment, found {count}") + return text.replace(old, new, 1) + + +def patch_gate(text: str) -> str: + """Harden migration context paths and GitHub Models endpoint detection.""" + old_migrations = '''\tif [ "${#sql_migration_dirs[@]}" -gt 0 ]; then +\t\tlocal head_sha_for_migration_context migration_context_dir +\t\thead_sha_for_migration_context="$(trim_whitespace "${PR_HEAD_SHA:-}")" +\t\tif [ -n "$head_sha_for_migration_context" ] && +\t\t\tis_valid_git_commit_sha "$head_sha_for_migration_context" && +\t\t\tgit rev-parse --verify --quiet "$head_sha_for_migration_context^{commit}" >/dev/null; then +\t\t\tfor migration_context_dir in "${sql_migration_dirs[@]}"; do +\t\t\t\tgit -c core.quotepath=false ls-tree -r --name-only "$head_sha_for_migration_context" -- "$migration_context_dir/" 2>/dev/null | +\t\t\t\t\tgrep -E '\\.sql$' || true +\t\t\tdone +\t\tfi +\tfi +''' + new_migrations = '''\tif [ "${#sql_migration_dirs[@]}" -gt 0 ]; then +\t\tlocal head_sha_for_migration_context migration_context_dir +\t\tlocal migration_context_file normalized_migration_context_file +\t\thead_sha_for_migration_context="$(trim_whitespace "${PR_HEAD_SHA:-}")" +\t\tif [ -n "$head_sha_for_migration_context" ] && +\t\t\tis_valid_git_commit_sha "$head_sha_for_migration_context" && +\t\t\tgit rev-parse --verify --quiet "$head_sha_for_migration_context^{commit}" >/dev/null; then +\t\t\tfor migration_context_dir in "${sql_migration_dirs[@]}"; do +\t\t\t\twhile IFS= read -r migration_context_file; do +\t\t\t\t\t[ -n "$migration_context_file" ] || continue +\t\t\t\t\tnormalized_migration_context_file="$( +\t\t\t\t\t\tnormalize_changed_file_path "$migration_context_file" +\t\t\t\t\t)" || continue +\t\t\t\t\tcase "$normalized_migration_context_file" in +\t\t\t\t\t"$migration_context_dir"/*.sql) +\t\t\t\t\t\tprintf '%s\\n' "$normalized_migration_context_file" +\t\t\t\t\t\t;; +\t\t\t\t\tesac +\t\t\t\tdone < <( +\t\t\t\t\tgit -c core.quotepath=false ls-tree -r --name-only \\ +\t\t\t\t\t\t"$head_sha_for_migration_context" -- "$migration_context_dir/" \\ +\t\t\t\t\t\t2>/dev/null || true +\t\t\t\t) +\t\t\tdone +\t\tfi +\tfi +''' + text = replace_once( + text, + old_migrations, + new_migrations, + "migration context normalization", + ) + + old_api_base = '''github_models_api_base_is_active() { +\tlocal api_base_file="${LLM_API_BASE_FILE:-}" +\tlocal api_base_file_label="LLM_API_BASE_FILE" +\t# Cross-provider fallback: when the primary scan uses direct-OpenAI, +\t# LLM_API_BASE_FILE is not set, but github_models/* fallback models +\t# route through the GitHub Models endpoint supplied by +\t# STRIX_GITHUB_MODELS_API_BASE_FILE. Recognise either source so that +\t# github_models_rate_limit_should_skip_same_model_retry correctly skips +\t# same-model retries for rate-limited cross-provider fallback models. +\tif [ -z "$api_base_file" ] && [ -n "${STRIX_GITHUB_MODELS_API_BASE_FILE:-}" ]; then +\t\tapi_base_file="$STRIX_GITHUB_MODELS_API_BASE_FILE" +\t\tapi_base_file_label="STRIX_GITHUB_MODELS_API_BASE_FILE" +\tfi + +\tif [ -z "$api_base_file" ]; then +\t\treturn 1 +\tfi + +\tlocal resolved_llm_api_base_file +\tif ! resolved_llm_api_base_file="$(resolve_trusted_input_file "$api_base_file_label" "$api_base_file" 2>/dev/null)"; then +\t\treturn 1 +\tfi + +\tlocal llm_api_base_value +\tllm_api_base_value="$(cat -- "$resolved_llm_api_base_file" 2>/dev/null)" || return 1 +\tllm_api_base_value="${llm_api_base_value%%/generateContent*}" +\tllm_api_base_value="${llm_api_base_value%%:generateContent*}" +\tllm_api_base_value="$(trim_whitespace "$llm_api_base_value")" +\tis_github_models_api_base "$llm_api_base_value" +} +''' + new_api_base = '''github_models_api_base_is_active() { +\tlocal api_base_file_label api_base_file +\tlocal resolved_llm_api_base_file llm_api_base_value + +\t# A cross-provider fallback can have both files configured at once: the +\t# primary provider remains in LLM_API_BASE_FILE while GitHub Models uses its +\t# dedicated endpoint file. Inspect the dedicated fallback endpoint first, +\t# then the primary endpoint, so either valid source activates the rate-limit +\t# retry short-circuit without allowing one non-GitHub value to hide the other. +\tfor api_base_file_label in STRIX_GITHUB_MODELS_API_BASE_FILE LLM_API_BASE_FILE; do +\t\tcase "$api_base_file_label" in +\t\tSTRIX_GITHUB_MODELS_API_BASE_FILE) +\t\t\tapi_base_file="${STRIX_GITHUB_MODELS_API_BASE_FILE:-}" +\t\t\t;; +\t\tLLM_API_BASE_FILE) +\t\t\tapi_base_file="${LLM_API_BASE_FILE:-}" +\t\t\t;; +\t\tesac +\t\t[ -n "$api_base_file" ] || continue +\t\tresolved_llm_api_base_file="$( +\t\t\tresolve_trusted_input_file "$api_base_file_label" "$api_base_file" 2>/dev/null +\t\t)" || continue +\t\tllm_api_base_value="$(cat -- "$resolved_llm_api_base_file" 2>/dev/null)" || continue +\t\tllm_api_base_value="${llm_api_base_value%%/generateContent*}" +\t\tllm_api_base_value="${llm_api_base_value%%:generateContent*}" +\t\tllm_api_base_value="$(trim_whitespace "$llm_api_base_value")" +\t\tif is_github_models_api_base "$llm_api_base_value"; then +\t\t\treturn 0 +\t\tfi +\tdone +\treturn 1 +} +''' + return replace_once( + text, + old_api_base, + new_api_base, + "GitHub Models endpoint source selection", + ) + + +def patch_tests(text: str) -> str: + """Strengthen the focused regression harness for all three review findings.""" + text = replace_once( + text, + '''\tassert_file_contains "$GATE_SCRIPT" "git -c core.quotepath=false ls-tree -r --name-only \\\"\\$head_sha_for_migration_context\\\" -- \\\"\\$migration_context_dir/\\\"" "strix gate enumerates sibling migrations from the PR head without quoting non-ASCII paths" +\tassert_file_contains "$GATE_SCRIPT" "fails open" "strix gate migration context enumeration is documented as fail-open" +''', + '''\tassert_file_contains "$GATE_SCRIPT" "git -c core.quotepath=false ls-tree -r --name-only \\\"\\$head_sha_for_migration_context\\\" -- \\\"\\$migration_context_dir/\\\"" "strix gate enumerates sibling migrations from the PR head without quoting non-ASCII paths" +\tassert_file_contains "$GATE_SCRIPT" 'normalize_changed_file_path "$migration_context_file"' "strix gate normalizes every sibling migration path before emission" +\tassert_file_contains "$GATE_SCRIPT" "for api_base_file_label in STRIX_GITHUB_MODELS_API_BASE_FILE LLM_API_BASE_FILE" "strix gate evaluates both dedicated fallback and primary GitHub Models endpoints" +\tassert_file_contains "$GATE_SCRIPT" "fails open" "strix gate migration context enumeration is documented as fail-open" +''', + "static migration and endpoint contracts", + ) + text = replace_once( + text, + '''\tlocal tmp_dir +\ttmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/strix-migration-context.XXXXXX")" +''', + '''\tlocal tmp_dir head_sha +\ttmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/strix-migration-context.XXXXXX")" +''', + "local head SHA declaration", + ) + text = replace_once( + text, + '''\t\tprintf 'ALTER TABLE t ADD COLUMN d text;\\n' >"server/db with space/migrations/0003_add_second_col.sql" +\t\tgit add -A +''', + '''\t\tprintf 'ALTER TABLE t ADD COLUMN d text;\\n' >"server/db with space/migrations/0003_add_second_col.sql" +\t\tprintf 'SELECT 1;\\n' >"server/db with space/migrations/0004_bad;name.sql" +\t\tgit add -A +''', + "unsafe migration fixture", + ) + text = replace_once( + text, + '''\t\t\t\tnormalize_changed_file_path() { printf "%s" "$1"; } +\t\t\t\t'"$(sed -n "/^pull_request_scope_context_files()/,/^}/p" "$GATE_SCRIPT")"' +''', + '''\t\t\t\t'"$(sed -n "/^normalize_changed_file_path()/,/^}/p" "$GATE_SCRIPT")"' +\t\t\t\t'"$(sed -n "/^pull_request_scope_context_files()/,/^}/p" "$GATE_SCRIPT")"' +''', + "functional path normalizer fixture", + ) + return replace_once( + text, + '''\tassert_file_contains "$tmp_dir/out.txt" "server/db with space/migrations/0001_기초.sql" "strix gate preserves spaces and non-ASCII sibling migration paths" +\tlocal sibling_count +''', + '''\tassert_file_contains "$tmp_dir/out.txt" "server/db with space/migrations/0001_기초.sql" "strix gate preserves spaces and non-ASCII sibling migration paths" +\tassert_file_not_contains "$tmp_dir/out.txt" "0004_bad;name.sql" "strix gate silently skips unsafe sibling migration paths" +\tlocal sibling_count +''', + "unsafe path regression assertion", + ) + + +def main() -> int: + """Patch reviewed fragments and remove the one-shot privileged bootstrap.""" + GATE_PATH.write_text(patch_gate(GATE_PATH.read_text(encoding="utf-8")), encoding="utf-8") + TEST_PATH.write_text(patch_tests(TEST_PATH.read_text(encoding="utf-8")), encoding="utf-8") + WORKFLOW_PATH.unlink(missing_ok=True) + SELF_PATH.unlink(missing_ok=True) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/ci/strix_quick_gate.sh b/scripts/ci/strix_quick_gate.sh index 3b001a921..597d709b1 100755 --- a/scripts/ci/strix_quick_gate.sh +++ b/scripts/ci/strix_quick_gate.sh @@ -1182,6 +1182,7 @@ pull_request_scope_context_files() { local needs_frontend_email_api_context=0 local needs_deployment_context=0 local changed_file normalized_changed_file + local -a sql_migration_dirs=() for changed_file in "$@"; do normalized_changed_file="$(normalize_changed_file_path "$changed_file")" || return 2 case "$normalized_changed_file" in @@ -1203,6 +1204,28 @@ pull_request_scope_context_files() { needs_deployment_context=1 ;; esac + # A single SQL migration references schema objects (tables, columns) that its + # sibling migrations create. Diff-scoping one migration in isolation makes the + # scanner report phantom "relation does not exist" / "table does not exist" + # findings — a false positive whose PoC depends only on files excluded from the + # scope. Collect each touched migrations directory so the whole ordered set is + # supplied from the PR head as read-only context. Generic across repositories; + # no project-specific paths are hard-coded. + case "$normalized_changed_file" in + */migrations/*.sql | migrations/*.sql) + local migration_dir="${normalized_changed_file%/*}" + local seen_migration_dir=0 known_migration_dir + for known_migration_dir in "${sql_migration_dirs[@]}"; do + if [ "$known_migration_dir" = "$migration_dir" ]; then + seen_migration_dir=1 + break + fi + done + if [ "$seen_migration_dir" -eq 0 ]; then + sql_migration_dirs+=("$migration_dir") + fi + ;; + esac done if [ "$needs_backend_python" -eq 1 ]; then @@ -1282,6 +1305,25 @@ render.yaml VERSION EOF fi + + # Enumerate sibling SQL migrations from the PR-head tree so cross-migration + # schema references resolve during the scan. Enumeration is read-only and + # fails open: when the head SHA is unavailable or invalid, no context is added + # and the base changed-file scan is unaffected. Changed migrations are copied + # from PR-head blobs; unchanged siblings come from the trusted materialized + # checkout. Both paths are normalized and supplied only as scanner context. + if [ "${#sql_migration_dirs[@]}" -gt 0 ]; then + local head_sha_for_migration_context migration_context_dir + head_sha_for_migration_context="$(trim_whitespace "${PR_HEAD_SHA:-}")" + if [ -n "$head_sha_for_migration_context" ] && + is_valid_git_commit_sha "$head_sha_for_migration_context" && + git rev-parse --verify --quiet "$head_sha_for_migration_context^{commit}" >/dev/null; then + for migration_context_dir in "${sql_migration_dirs[@]}"; do + git -c core.quotepath=false ls-tree -r --name-only "$head_sha_for_migration_context" -- "$migration_context_dir/" 2>/dev/null | + grep -E '\.sql$' || true + done + fi + fi } changed_file_list_contains() { @@ -2766,12 +2808,25 @@ is_vertex_not_found_error() { } github_models_api_base_is_active() { - if [ -z "$LLM_API_BASE_FILE" ]; then + local api_base_file="${LLM_API_BASE_FILE:-}" + local api_base_file_label="LLM_API_BASE_FILE" + # Cross-provider fallback: when the primary scan uses direct-OpenAI, + # LLM_API_BASE_FILE is not set, but github_models/* fallback models + # route through the GitHub Models endpoint supplied by + # STRIX_GITHUB_MODELS_API_BASE_FILE. Recognise either source so that + # github_models_rate_limit_should_skip_same_model_retry correctly skips + # same-model retries for rate-limited cross-provider fallback models. + if [ -z "$api_base_file" ] && [ -n "${STRIX_GITHUB_MODELS_API_BASE_FILE:-}" ]; then + api_base_file="$STRIX_GITHUB_MODELS_API_BASE_FILE" + api_base_file_label="STRIX_GITHUB_MODELS_API_BASE_FILE" + fi + + if [ -z "$api_base_file" ]; then return 1 fi local resolved_llm_api_base_file - if ! resolved_llm_api_base_file="$(resolve_trusted_input_file "LLM_API_BASE_FILE" "$LLM_API_BASE_FILE" 2>/dev/null)"; then + if ! resolved_llm_api_base_file="$(resolve_trusted_input_file "$api_base_file_label" "$api_base_file" 2>/dev/null)"; then return 1 fi diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index b4d585b9e..1cada35c4 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -173,6 +173,53 @@ assert_strix_pr_scope_includes_deployment_context() { assert_file_contains "$GATE_SCRIPT" "scripts/ci/test_*.sh" "strix gate excludes large CI self-test harnesses from PR scan targets" } +assert_strix_pr_scope_includes_sql_migration_context() { + assert_file_contains "$GATE_SCRIPT" "*/migrations/*.sql | migrations/*.sql" "strix gate recognizes SQL migration files for sibling context" + assert_file_contains "$GATE_SCRIPT" "sql_migration_dirs+=(\"\$migration_dir\")" "strix gate collects each touched migrations directory" + assert_file_contains "$GATE_SCRIPT" "git -c core.quotepath=false ls-tree -r --name-only \"\$head_sha_for_migration_context\" -- \"\$migration_context_dir/\"" "strix gate enumerates sibling migrations from the PR head without quoting non-ASCII paths" + assert_file_contains "$GATE_SCRIPT" "fails open" "strix gate migration context enumeration is documented as fail-open" +} + +assert_strix_pr_scope_migration_siblings_functional() { + # A migration-only diff must resolve schema references from sibling migrations, + # not report a phantom "relation does not exist" finding. Enumerate the context + # a three-migration directory produces when the second and third files change. + local tmp_dir + tmp_dir="$(mktemp -d "${TMPDIR:-/tmp}/strix-migration-context.XXXXXX")" + ( + cd "$tmp_dir" + git init -q + git config user.email ci@example.com + git config user.name ci + mkdir -p "server/db with space/migrations" + printf 'CREATE TABLE t (id int);\n' >"server/db with space/migrations/0001_기초.sql" + printf 'ALTER TABLE t ADD COLUMN c text;\n' >"server/db with space/migrations/0002_add_col.sql" + printf 'ALTER TABLE t ADD COLUMN d text;\n' >"server/db with space/migrations/0003_add_second_col.sql" + git add -A + git commit -qm base + head_sha="$(git rev-parse HEAD)" + + # shellcheck source=/dev/null + REPO_ROOT="$tmp_dir" PR_HEAD_SHA="$head_sha" \ + bash -c ' + set -euo pipefail + REPO_ROOT="'"$tmp_dir"'" + trim_whitespace() { printf "%s" "$1"; } + is_valid_git_commit_sha() { [[ "$1" =~ ^[0-9a-fA-F]{40}$ ]]; } + normalize_changed_file_path() { printf "%s" "$1"; } + '"$(sed -n "/^pull_request_scope_context_files()/,/^}/p" "$GATE_SCRIPT")"' + pull_request_scope_context_files \ + "server/db with space/migrations/0002_add_col.sql" \ + "server/db with space/migrations/0003_add_second_col.sql" + ' >"$tmp_dir/out.txt" + ) + assert_file_contains "$tmp_dir/out.txt" "server/db with space/migrations/0001_기초.sql" "strix gate preserves spaces and non-ASCII sibling migration paths" + local sibling_count + sibling_count="$(grep -Fc "server/db with space/migrations/0001_기초.sql" "$tmp_dir/out.txt" || true)" + assert_equals "1" "$sibling_count" "strix gate deduplicates a migration directory containing spaces" + rm -rf "$tmp_dir" +} + assert_strix_workflow_pr_trigger_hardened() { local workflow_file="$REPO_ROOT/.github/workflows/strix.yml" @@ -3384,6 +3431,31 @@ REPORT ;; esac ;; + openai-direct-github-models-fallback-ratelimit-skip) + # Primary openai_direct fails; first github_models fallback hits GitHub + # Models rate limit. The gate must detect the GitHub Models API base via + # STRIX_GITHUB_MODELS_API_BASE_FILE and skip same-model retries of the + # rate-limited fallback, moving directly to the second fallback. + case "${STRIX_LLM:-}" in + openai/gpt-5.6-luna) + echo "Error getting response: Error code: 429 - {'error': {'message': 'You exceeded your current quota, please check your plan and billing details.', 'type': 'insufficient_quota', 'code': 'insufficient_quota'}}" + echo "openai.RateLimitError: Error code: 429" + exit 1 + ;; + openai/gpt-5-chat) + echo "RateLimitError: Too many requests. For more on scraping GitHub and how it may affect your rights, please review our Terms of Service (https://docs.github.com/en/site-policy/github-terms/github-terms-of-service)." + exit 1 + ;; + openai/o3) + echo "scan ok with second GitHub Models fallback" + exit 0 + ;; + *) + echo "unexpected model ${STRIX_LLM:-}" >&2 + exit 9 + ;; + esac + ;; vertex-all-notfound) echo "Error: litellm.NotFoundError: Vertex_aiException - x" echo '"status": "NOT_FOUND"' @@ -5495,7 +5567,8 @@ PY FAKE_STRIX_OUTSIDE_REPORT_DIR="$repo_root_dir/outside-strix-report" ) fi - if [ "$scenario" = "openai-direct-quota-github-models-fallback-success" ]; then + if [ "$scenario" = "openai-direct-quota-github-models-fallback-success" ] || + [ "$scenario" = "openai-direct-github-models-fallback-ratelimit-skip" ]; then printf '%s' 'https://models.github.ai/inference' >"$tmp_dir/github_models_api_base.txt" printf '%s' 'github-models-fallback-token' >"$tmp_dir/github_models_key.txt" env_cmd+=(STRIX_GITHUB_MODELS_API_BASE_FILE="$tmp_dir/github_models_api_base.txt") @@ -5691,6 +5764,17 @@ PY "scenario=$scenario does not sleep in same-model retry after GitHub Models rate limiting" fi + if [ "$scenario" = "openai-direct-github-models-fallback-ratelimit-skip" ]; then + assert_file_contains \ + "$output_log" \ + "GitHub Models rate limit detected for model 'github_models/openai/gpt-5-chat'; skipping same-model retry and moving directly to fallback models or current-head neutral classification." \ + "scenario=$scenario logs why same-model retry was skipped for cross-provider fallback" + assert_file_not_contains \ + "$output_log" \ + "Retrying model 'github_models/openai/gpt-5-chat' due to rate limit" \ + "scenario=$scenario does not retry rate-limited cross-provider fallback model" + fi + if [ "$scenario" = "pr-changed-scope-full-set" ]; then assert_internal_pr_scope_targets "$target_log" "$repo_root_dir" "$expected_calls" fi @@ -5897,6 +5981,36 @@ run_filtered_gate_case_if_requested() { "" \ "github_models/openai/o3" ;; + openai-direct-github-models-fallback-ratelimit-skip) + run_gate_case "openai-direct-github-models-fallback-ratelimit-skip" \ + "openai_direct/gpt-5.6-luna" \ + "" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'github_models/openai/o3' in [0-9]+s\\." \ + "5" \ + "openai/gpt-5.6-luna|openai/gpt-5.6-luna|openai/gpt-5.6-luna|openai/gpt-5-chat|openai/o3" \ + "|||https://models.github.ai/inference|https://models.github.ai/inference" \ + "vertex_ai" \ + "" \ + "" \ + "2" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "github_models/openai/gpt-5-chat github_models/openai/o3" + ;; gemini-timeout-fallback-success) run_gate_case_allow_provider_signal "gemini-timeout-fallback-success" \ "gemini/timeout-fallback-primary" \ @@ -8837,6 +8951,10 @@ assert_strix_workflow_pr_trigger_hardened assert_strix_pr_scope_includes_deployment_context +assert_strix_pr_scope_includes_sql_migration_context + +assert_strix_pr_scope_migration_siblings_functional + assert_strix_gpt54_model_guard_cases assert_strix_gate_target_scope_separated @@ -11966,6 +12084,41 @@ run_gate_case "openai-direct-quota-github-models-fallback-success" \ "" \ "github_models/openai/o3" +# Direct-OpenAI primary hits a quota/rate-limit error (retries 3x, as +# openai_direct is not subject to the GitHub Models skip), then the first +# github_models fallback immediately hits a GitHub Models rate limit. +# The gate must detect GitHub Models is active via STRIX_GITHUB_MODELS_API_BASE_FILE +# (not LLM_API_BASE_FILE, which is unset for openai_direct) and skip same-model retries +# of the rate-limited fallback, moving directly to the second fallback (5 calls total). +run_gate_case "openai-direct-github-models-fallback-ratelimit-skip" \ + "openai_direct/gpt-5.6-luna" \ + "" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'github_models/openai/o3' in [0-9]+s\\." \ + "5" \ + "openai/gpt-5.6-luna|openai/gpt-5.6-luna|openai/gpt-5.6-luna|openai/gpt-5-chat|openai/o3" \ + "|||https://models.github.ai/inference|https://models.github.ai/inference" \ + "vertex_ai" \ + "" \ + "" \ + "2" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "" \ + "github_models/openai/gpt-5-chat github_models/openai/o3" + run_gate_case "github-models-fallback-success-deepseek-v3" \ "vertex_ai/missing-primary" \ "github_models/deepseek/deepseek-r1-0528 github_models/deepseek/deepseek-v3-0324" \