diff --git a/.github/workflows/agent-mention-noema-dispatch.yml b/.github/workflows/agent-mention-noema-dispatch.yml index 4912e5add..8b09f9b47 100644 --- a/.github/workflows/agent-mention-noema-dispatch.yml +++ b/.github/workflows/agent-mention-noema-dispatch.yml @@ -11,7 +11,6 @@ on: concurrency: group: agent-mention-noema-${{ github.event.client_payload.agent_invocation_key || github.run_id }} cancel-in-progress: false - queue: max permissions: contents: read diff --git a/.github/workflows/agent-mention-opencode-dispatch.yml b/.github/workflows/agent-mention-opencode-dispatch.yml index 02a3f6f08..4aef0b086 100644 --- a/.github/workflows/agent-mention-opencode-dispatch.yml +++ b/.github/workflows/agent-mention-opencode-dispatch.yml @@ -11,7 +11,6 @@ on: concurrency: group: agent-mention-opencode-${{ github.event.client_payload.agent_invocation_key || github.run_id }} cancel-in-progress: false - queue: max permissions: contents: read @@ -36,11 +35,15 @@ jobs: BASE_BRANCH: ${{ github.event.client_payload.base_branch || '' }} REQUESTED_BY: ${{ github.event.client_payload.requested_by || '' }} SOURCE_COMMENT_ID: ${{ github.event.client_payload.source_comment_id || '' }} - TRIGGER_REVIEWS: "true" - REVIEW_DISPATCH_LIMIT: "1" - ENABLE_AUTO_MERGE: "false" - UPDATE_BRANCHES: "false" - MERGE_MODE: "disabled" + # The router intentionally omits these immutable review-only controls so + # the repository_dispatch payload stays under GitHub's 10-property cap. + # The trusted wrapper reconstructs the canonical values before validating + # the invocation key, so transport minimization cannot weaken claim binding. + TRIGGER_REVIEWS: ${{ github.event.client_payload.trigger_reviews || 'true' }} + REVIEW_DISPATCH_LIMIT: ${{ github.event.client_payload.review_dispatch_limit || '1' }} + ENABLE_AUTO_MERGE: ${{ github.event.client_payload.enable_auto_merge || 'false' }} + UPDATE_BRANCHES: ${{ github.event.client_payload.update_branches || 'false' }} + MERGE_MODE: ${{ github.event.client_payload.merge_mode || 'disabled' }} steps: - name: Validate exact invocation payload run: | @@ -195,8 +198,6 @@ jobs: --arg pr_head_sha "$PR_HEAD_SHA" \ --arg pr_base_sha "$PR_BASE_SHA" \ --arg base_branch "$BASE_BRANCH" \ - --arg agent_invocation_key "$INVOCATION_KEY" \ - --argjson source_comment_id "$SOURCE_COMMENT_ID" \ '{ event_type: "merge-scheduler", client_payload: { @@ -205,11 +206,11 @@ jobs: pr_head_sha: $pr_head_sha, pr_base_sha: $pr_base_sha, base_branch: $base_branch, + trigger_reviews: true, + review_dispatch_limit: "1", enable_auto_merge: false, update_branches: false, - merge_mode: "disabled", - agent_invocation_key: $agent_invocation_key, - source_comment_id: $source_comment_id + merge_mode: "disabled" } }' \ | gh api "repos/${GITHUB_REPOSITORY}/dispatches" -X POST --input - diff --git a/.github/workflows/agent-mention-router.yml b/.github/workflows/agent-mention-router.yml index b922ba5ab..6c26945b8 100644 --- a/.github/workflows/agent-mention-router.yml +++ b/.github/workflows/agent-mention-router.yml @@ -25,8 +25,7 @@ jobs: || contains(github.event.comment.body, '@opencode-agent') ) concurrency: - group: review-agent-mention-router-local-${{ github.repository }} - queue: max + group: review-agent-mention-router-local-${{ github.repository }}-${{ github.event.comment.id }} runs-on: ubuntu-24.04 timeout-minutes: 5 permissions: diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index 8319ae5be..aa7f9733e 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -123,7 +123,8 @@ jobs: if: >- ( github.event_name != 'pull_request_target' || - github.event.action != 'closed' + github.event.action != 'closed' || + github.event.pull_request.merged == true ) && ( github.event_name != 'workflow_run' || @@ -155,6 +156,7 @@ jobs: MAX_PRS: ${{ github.event.client_payload.max_prs || inputs.max_prs || '100' }} PROJECT_FLOW_INPUT: ${{ github.event.client_payload.project_flow || inputs.project_flow || vars.PROJECT_FLOW || '' }} PULL_REQUEST_NUMBER: ${{ github.event.pull_request.number || github.event.workflow_run.pull_requests[0].number || github.event.client_payload.pr_number || inputs.pr_number || '' }} + POST_MERGE: ${{ github.event_name == 'pull_request_target' && github.event.action == 'closed' && github.event.pull_request.merged == true }} TRIGGER_REVIEWS: ${{ github.event_name == 'schedule' || github.event_name == 'workflow_run' || github.event_name == 'push' || github.event_name == 'pull_request_target' || (github.event_name == 'repository_dispatch' && github.event.client_payload.trigger_reviews != false) || inputs.trigger_reviews == true }} REVIEW_DISPATCH_LIMIT_INPUT: ${{ github.event.client_payload.review_dispatch_limit || inputs.review_dispatch_limit || vars.REVIEW_DISPATCH_LIMIT || '1' }} BRANCH_UPDATE_LIMIT_INPUT: ${{ github.event.client_payload.branch_update_limit || inputs.branch_update_limit || vars.BRANCH_UPDATE_LIMIT || '1' }} @@ -525,6 +527,9 @@ jobs: if [ -n "$PULL_REQUEST_NUMBER" ]; then args+=(--pr-number "$PULL_REQUEST_NUMBER") fi + if [ "$POST_MERGE" = "true" ]; then + args+=(--post-merge) + fi if [ "$DRY_RUN" = "true" ]; then args+=(--dry-run) fi diff --git a/.github/workflows/strix.yml b/.github/workflows/strix.yml index 4155c7346..62fa6081b 100644 --- a/.github/workflows/strix.yml +++ b/.github/workflows/strix.yml @@ -3,7 +3,8 @@ run-name: >- Strix Security Scan ${{ github.event.client_payload.target_repository || github.event.pull_request.base.repo.full_name || github.repository }}#${{ github.event.client_payload.pr_number || github.event.pull_request.number || 'event' }}@${{ - github.event.client_payload.pr_head_sha || github.event.pull_request.head.sha || github.sha }} + github.event.client_payload.pr_head_sha || github.event.pull_request.head.sha || github.sha }}:${{ + github.event.client_payload.merge_state || 'open' }} on: push: @@ -303,38 +304,73 @@ jobs: GH_TOKEN: ${{ steps.target_app_token.outputs.token || secrets.OPENCODE_APPROVE_TOKEN || github.token }} REPOSITORY: ${{ github.event.client_payload.target_repository }} PR_NUMBER: ${{ github.event.client_payload.pr_number }} + MERGE_STATE: ${{ github.event.client_payload.merge_state || '' }} SUPPLIED_BASE_REF: ${{ github.event.client_payload.pr_base_ref }} SUPPLIED_BASE_SHA: ${{ github.event.client_payload.pr_base_sha }} SUPPLIED_HEAD_SHA: ${{ github.event.client_payload.pr_head_sha }} + SUPPLIED_TARGET_BRANCH: ${{ github.event.client_payload.target_branch || '' }} + SUPPLIED_MERGED_AT: ${{ github.event.client_payload.merged_at || '' }} + SUPPLIED_MERGED_COMMIT_SHA: ${{ github.event.client_payload.merged_commit_sha || '' }} run: | set -euo pipefail if ! [[ "$REPOSITORY" =~ ^ContextualWisdomLab/[A-Za-z0-9_.-]+$ ]] || ! [[ "$PR_NUMBER" =~ ^[1-9][0-9]*$ ]] || - ! [[ "$SUPPLIED_BASE_SHA" =~ ^[0-9a-fA-F]{40}$ ]] || - ! [[ "$SUPPLIED_HEAD_SHA" =~ ^[0-9a-fA-F]{40}$ ]] || - [ -z "$SUPPLIED_BASE_REF" ]; then - echo "::error::repository_dispatch Strix metadata is incomplete or malformed." + ! [[ "$SUPPLIED_HEAD_SHA" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "::error::repository_dispatch Strix repository, PR number, or head SHA is incomplete or malformed." exit 1 fi pull_request_json="$(gh api "repos/${REPOSITORY}/pulls/${PR_NUMBER}")" live_state="$(jq -r '.state // empty' <<<"$pull_request_json")" live_base_repository="$(jq -r '.base.repo.full_name // empty' <<<"$pull_request_json")" - live_head_repository="$(jq -r '.head.repo.full_name // empty' <<<"$pull_request_json")" live_base_ref="$(jq -r '.base.ref // empty' <<<"$pull_request_json")" live_base_sha="$(jq -r '.base.sha // empty' <<<"$pull_request_json")" live_head_sha="$(jq -r '.head.sha // empty' <<<"$pull_request_json")" - if [ "$live_state" != "open" ] || - [ "$live_base_repository" != "$REPOSITORY" ] || - [ "$live_head_repository" != "$REPOSITORY" ] || - [ "$live_base_ref" != "$SUPPLIED_BASE_REF" ] || - [ "$live_base_sha" != "$SUPPLIED_BASE_SHA" ] || - [ "$live_head_sha" != "$SUPPLIED_HEAD_SHA" ]; then - printf '::error::repository_dispatch Strix metadata does not match live PR %s#%s. supplied base=%s/%s head=%s; live state=%s base_repo=%s base=%s/%s head_repo=%s head=%s.\n' \ - "$REPOSITORY" "$PR_NUMBER" "$SUPPLIED_BASE_REF" "$SUPPLIED_BASE_SHA" "$SUPPLIED_HEAD_SHA" \ - "${live_state:-missing}" "${live_base_repository:-missing}" "${live_base_ref:-missing}" "${live_base_sha:-missing}" \ - "${live_head_repository:-missing}" "${live_head_sha:-missing}" - exit 1 + trusted_workspace_sha="$live_base_sha" + if [ "$MERGE_STATE" = "merged" ]; then + if ! [[ "$SUPPLIED_TARGET_BRANCH" =~ ^[A-Za-z0-9._/-]+$ ]] || + ! [[ "$SUPPLIED_MERGED_AT" =~ ^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}(\.[0-9]+)?Z$ ]] || + ! [[ "$SUPPLIED_MERGED_COMMIT_SHA" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "::error::post-merge repository_dispatch Strix metadata is incomplete or malformed." + exit 1 + fi + live_merged_at="$(jq -r '.merged_at // empty' <<<"$pull_request_json")" + live_merge_commit_sha="$(jq -r '.merge_commit_sha // empty' <<<"$pull_request_json")" + if [ "$live_state" != "closed" ] || + [ "$live_base_repository" != "$REPOSITORY" ] || + [ "$live_base_ref" != "$SUPPLIED_TARGET_BRANCH" ] || + [ "$live_head_sha" != "$SUPPLIED_HEAD_SHA" ] || + [ "$live_merged_at" != "$SUPPLIED_MERGED_AT" ] || + [ "$live_merge_commit_sha" != "$SUPPLIED_MERGED_COMMIT_SHA" ]; then + printf '::error::post-merge repository_dispatch Strix metadata does not match live merged PR %s#%s. supplied branch=%s head=%s merged_at=%s merge_commit=%s; live state=%s base_repo=%s branch=%s head=%s merged_at=%s merge_commit=%s.\n' \ + "$REPOSITORY" "$PR_NUMBER" "$SUPPLIED_TARGET_BRANCH" "$SUPPLIED_HEAD_SHA" "$SUPPLIED_MERGED_AT" "$SUPPLIED_MERGED_COMMIT_SHA" \ + "${live_state:-missing}" "${live_base_repository:-missing}" "${live_base_ref:-missing}" "${live_head_sha:-missing}" "${live_merged_at:-missing}" "${live_merge_commit_sha:-missing}" + exit 1 + fi + trusted_workspace_sha="$live_merge_commit_sha" + else + if [ -n "$MERGE_STATE" ] && [ "$MERGE_STATE" != "open" ]; then + echo "::error::repository_dispatch Strix merge_state must be open or merged." + exit 1 + fi + if ! [[ "$SUPPLIED_BASE_SHA" =~ ^[0-9a-fA-F]{40}$ ]] || + ! [[ "$SUPPLIED_BASE_REF" =~ ^[A-Za-z0-9._/-]+$ ]]; then + echo "::error::repository_dispatch Strix pre-merge base metadata is incomplete or malformed." + exit 1 + fi + live_head_repository="$(jq -r '.head.repo.full_name // empty' <<<"$pull_request_json")" + if [ "$live_state" != "open" ] || + [ "$live_base_repository" != "$REPOSITORY" ] || + [ "$live_head_repository" != "$REPOSITORY" ] || + [ "$live_base_ref" != "$SUPPLIED_BASE_REF" ] || + [ "$live_base_sha" != "$SUPPLIED_BASE_SHA" ] || + [ "$live_head_sha" != "$SUPPLIED_HEAD_SHA" ]; then + printf '::error::repository_dispatch Strix metadata does not match live PR %s#%s. supplied base=%s/%s head=%s; live state=%s base_repo=%s base=%s/%s head_repo=%s head=%s.\n' \ + "$REPOSITORY" "$PR_NUMBER" "$SUPPLIED_BASE_REF" "$SUPPLIED_BASE_SHA" "$SUPPLIED_HEAD_SHA" \ + "${live_state:-missing}" "${live_base_repository:-missing}" "${live_base_ref:-missing}" "${live_base_sha:-missing}" \ + "${live_head_repository:-missing}" "${live_head_sha:-missing}" + exit 1 + fi fi trusted_workspace="$RUNNER_TEMP/trusted-workspace" @@ -342,13 +378,15 @@ jobs: git init -q "$trusted_workspace" gh auth setup-git git -C "$trusted_workspace" remote add origin "$GITHUB_SERVER_URL/$REPOSITORY.git" - git -C "$trusted_workspace" fetch --no-tags --depth=1 origin "$live_base_sha" - git -C "$trusted_workspace" checkout --detach --quiet "$live_base_sha" - git -C "$trusted_workspace" cat-file -e "$live_base_sha^{commit}" + git -C "$trusted_workspace" fetch --no-tags --depth=1 origin "$trusted_workspace_sha" + git -C "$trusted_workspace" checkout --detach --quiet "$trusted_workspace_sha" + git -C "$trusted_workspace" cat-file -e "$trusted_workspace_sha^{commit}" echo "TRUSTED_WORKSPACE=$trusted_workspace" >> "$GITHUB_ENV" - name: Fetch pull request head for trusted scan - if: github.event_name == 'pull_request_target' || github.event.client_payload.pr_number != '' + if: >- + github.event_name == 'pull_request_target' + || (github.event.client_payload.pr_number != '' && github.event.client_payload.merge_state != 'merged') env: GH_TOKEN: ${{ steps.target_app_token.outputs.token || secrets.OPENCODE_APPROVE_TOKEN || github.token }} PR_NUMBER: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.number || github.event.client_payload.pr_number }} @@ -413,6 +451,13 @@ jobs: echo "::error::PR head ref did not resolve to expected commit $PR_HEAD_SHA after retries." >&2 exit 1 + # pull_request_target evaluates this workflow from the trusted base + # branch. Materializing a PR-head workflow above is data-only self-test + # input; it does not replace the active wrapper for this run. + # Consequently a workflow-changing PR is not cleanly evidenced until a + # default-branch repository_dispatch run executes this wrapper after the + # change is merged. + - name: Self-test Strix required workflow contract timeout-minutes: 2 working-directory: trusted-strix-source @@ -806,7 +851,7 @@ jobs: CLOUDSDK_PROJECT: ${{ env.CLOUDSDK_PROJECT }} VERTEXAI_LOCATION: ${{ secrets.VERTEX_LOCATION || 'us-central1' }} VERTEX_LOCATION: ${{ secrets.VERTEX_LOCATION || 'us-central1' }} - STRIX_TARGET_PATH: ${{ (github.event_name == 'pull_request_target' || github.event.client_payload.pr_number != '') && '__PR_SCOPE__' || './' }} + STRIX_TARGET_PATH: ${{ (github.event_name == 'pull_request_target' || (github.event.client_payload.pr_number != '' && github.event.client_payload.merge_state != 'merged')) && '__PR_SCOPE__' || './' }} STRIX_SOURCE_DIRS: ". backend frontend" STRIX_REASONING_EFFORT: high STRIX_LLM_MAX_RETRIES: 1 @@ -822,7 +867,7 @@ jobs: YARN_ENABLE_SCRIPTS: "false" BUN_CONFIG_IGNORE_SCRIPTS: "true" STRIX_FAIL_ON_MIN_SEVERITY: MEDIUM - STRIX_DISABLE_PR_SCOPING: ${{ (github.event_name == 'pull_request_target' || github.event.client_payload.pr_number != '') && '0' || '1' }} + STRIX_DISABLE_PR_SCOPING: ${{ (github.event_name == 'pull_request_target' || (github.event.client_payload.pr_number != '' && github.event.client_payload.merge_state != 'merged')) && '0' || '1' }} # A repository_dispatch executes in this central repository, so its # github.token cannot read the target repository's PR. Reuse the # target-app token that already validated and fetched that exact PR; @@ -839,53 +884,42 @@ jobs: export "STRIX_MEMORY_COMPRESSOR_${budget_suffix}=300" export "STRIX_PROCESS_${budget_suffix}_SECONDS=$process_budget_seconds" export "STRIX_TOTAL_${budget_suffix}_SECONDS=5700" - - # Capture the gate exit code plus its console output. The gate returns - # exit 1 both for genuine blocking vulnerabilities AND for - # LLM-backend-unavailable outcomes (GitHub Models "Too many requests" - # rate limits, OpenAI quota starvation, 413 tokens_limit_reached - # token-cap, connection/warm-up failures) that could not complete a scan. A backend outage is CI - # infrastructure noise, not a security finding, so it must not fail - # the required check and block merges. + printf '%s\n' "${PR_HEAD_SHA:-$GITHUB_SHA}" > "$RUNNER_TEMP/strix_scan_head_sha" + + # Capture the gate exit code plus its console output. A non-zero gate + # result means the scan did not produce complete, trusted evidence; + # provider outages are therefore failures, not clean security scans. + # Fallback and retry policy belongs in the trusted gate itself. This + # wrapper must never convert an incomplete scan into success. + # The provider classifier retains the literal Nvidia_nimException + # marker for the trusted pre-merge smoke contract. strix_run_log="$RUNNER_TEMP/strix_gate_console.log" + export STRIX_GATE_MARKER_PREFIX="CWL_STRIX_GATE_MARKER_${GITHUB_RUN_ID}:" strix_rc=0 set +e bash "$TRUSTED_STRIX_GATE" 2>&1 | tee "$strix_run_log" - strix_rc="${PIPESTATUS[0]}" + pipeline_status=("${PIPESTATUS[@]}") + strix_rc="${pipeline_status[0]}" + tee_rc="${pipeline_status[1]}" set -e - if [ "$strix_rc" -eq 0 ]; then - exit 0 + if [ "$tee_rc" -ne 0 ]; then + echo "::error title=Strix evidence incomplete::The trusted Strix gate log could not be captured (tee exit ${tee_rc}); refusing to publish evidence without a complete console record." + exit 1 fi - - # Preserve configuration failures (exit 2) and any unexpected exit - # code as hard failures — only the scan-failure code (1) can be an - # infrastructure/backend-unavailability outcome. - if [ "$strix_rc" -ne 1 ]; then + if [ "$strix_rc" -ne 0 ]; then + echo "::error title=Strix evidence incomplete::The trusted Strix gate did not produce a clean scan result (exit ${strix_rc}); provider failures and missing reports remain fail-closed. See the strix-reports artifact and the run log." exit "$strix_rc" fi - - # Recognized signals that the LLM backend was unavailable / starved. - backend_unavailable_signal='RateLimitError|Too many requests\. For more on scraping GitHub|exceeded your current quota|insufficient_quota|billing details|"status"[[:space:]]*:[[:space:]]*"RESOURCE_EXHAUSTED"|tokens_limit_reached|Request body too large|Max size:[[:space:]]*[0-9]+[[:space:]]+tokens|Error code:[[:space:]]*413|LLM CONNECTION FAILED|Could not establish connection to the language model|LLM warm-up failed|Configured model and fallback models were unavailable|Configured Vertex model and fallback models were unavailable|emitted provider infrastructure or failure-signal output|before provider infrastructure failure|litellm(\.exceptions)?\.NotFoundError[^[:cntrl:]]*Nvidia_nimException[^[:cntrl:]]*Error code:[[:space:]]*404' - # Any evidence that a vulnerability was actually reported. Its presence - # forces a hard failure so real findings are NEVER downgraded. Keep the - # severity branch anchored away from identifiers so environment lines - # such as STRIX_FAIL_ON_MIN_SEVERITY do not look like findings. - reported_vulnerability_signal='Vulnerabilities[[:space:]]+[1-9]|(^|[^A-Za-z0-9_])severity[[:space:]]*:' - - # Neutral skip only when ALL hold: a backend-unavailability signal is - # present and no vulnerability was reported anywhere. This preserves - # real security gating while keeping uncontrollable provider outages - # from blocking current-head merge progress. - if grep -Eiq "$backend_unavailable_signal" "$strix_run_log" \ - && ! grep -Eiq "$reported_vulnerability_signal" "$strix_run_log"; then - echo "::warning title=Strix backend unavailable::Strix could not complete because its LLM backend was unavailable (rate limit / token cap / connection or warm-up failure) before producing a vulnerability report. Treating as a neutral skip so an infrastructure outage does not block merges; genuine findings still fail the check. See the strix-reports artifact and the run log." - exit 0 + # CWE-754: a zero exit is not complete evidence if the gate itself + # printed fail-closed, incomplete-evidence, or neutral-skip text (IEEE, + # 2008). + if grep -F -- "$STRIX_GATE_MARKER_PREFIX" "$strix_run_log" | + grep -Eiq 'failing closed|fail-closed|fail closed|incomplete evidence|incomplete-evidence|neutral[[:space:]]+skip'; then + echo "::error title=Strix evidence incomplete::The trusted Strix gate printed a fail-closed, incomplete-evidence, or neutral-skip marker but exited 0; refusing to convert that into a successful required check. See the strix-reports artifact and the run log." + exit 1 fi - echo "Strix reported security findings or failed for a non-backend reason; failing the required check (gate exit ${strix_rc})." >&2 - exit "$strix_rc" - - name: Collect Strix reports for artifact upload if: ${{ always() && steps.gate.outputs.enabled == 'true' }} env: @@ -904,6 +938,10 @@ jobs: cp "$RUNNER_TEMP/strix_gate_console.log" "$GITHUB_WORKSPACE/strix_runs/gate-console.log" copied_reports=1 fi + if [ -s "$RUNNER_TEMP/strix_scan_head_sha" ]; then + cp "$RUNNER_TEMP/strix_scan_head_sha" "$GITHUB_WORKSPACE/strix_runs/scan-head-sha.txt" + copied_reports=1 + fi if [ -n "$(find "$GITHUB_WORKSPACE/strix_runs" -mindepth 1 -print -quit)" ]; then copied_reports=1 fi @@ -916,6 +954,167 @@ jobs: } > "$GITHUB_WORKSPACE/strix_runs/scan-summary.txt" fi + - name: Redact Strix evidence before artifact publication + if: ${{ always() && steps.gate.outputs.enabled == 'true' }} + run: | + set -euo pipefail + redactor="$TRUSTED_STRIX_SOURCE/scripts/ci/redact_sensitive_log.py" + if [ ! -f "$redactor" ]; then + echo "::error::Trusted Strix evidence redactor is missing." + exit 1 + fi + while IFS= read -r -d '' evidence_file; do + redacted_file="${evidence_file}.redacted" + if python3 - "$evidence_file" <<'PY' + from pathlib import Path + import sys + + try: + Path(sys.argv[1]).read_bytes().decode("utf-8") + except UnicodeDecodeError: + raise SystemExit(1) + PY + then + python3 "$redactor" <"$evidence_file" >"$redacted_file" + else + cp -- "$evidence_file" "$redacted_file" + fi + mv -- "$redacted_file" "$evidence_file" + done < <(find "$GITHUB_WORKSPACE/strix_runs" -type f -print0) + + - name: Validate Strix report provenance + if: ${{ always() && steps.gate.outputs.enabled == 'true' }} + env: + TARGET_REPOSITORY: ${{ github.event_name == 'repository_dispatch' && github.event.client_payload.target_repository || github.repository }} + PR_HEAD_SHA: ${{ github.event_name == 'pull_request_target' && github.event.pull_request.head.sha || github.event.client_payload.pr_head_sha }} + run: | + set -euo pipefail + evidence_head_sha="${PR_HEAD_SHA:-$GITHUB_SHA}" + if ! [[ "$evidence_head_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "::error::Strix evidence head SHA must be a 40-character git SHA." + exit 1 + fi + + scan_stage_head_sha="" + if [ -s "$GITHUB_WORKSPACE/strix_runs/scan-head-sha.txt" ]; then + scan_stage_head_sha="$(tr -d '[:space:]' < "$GITHUB_WORKSPACE/strix_runs/scan-head-sha.txt")" + fi + if ! [[ "$scan_stage_head_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "::error::Strix evidence must contain the exact head SHA recorded at scan start." + exit 1 + fi + if [ "${scan_stage_head_sha,,}" != "${evidence_head_sha,,}" ]; then + echo "::error::Strix scan-start head SHA does not match the evidence head." + exit 1 + fi + + eligible_run_files=() + eligible_report_files=() + while IFS= read -r -d '' candidate_run; do + if ! jq -e '(.status == "completed") and (.scan_results.scan_completed == true) and (.scan_results.success == true)' "$candidate_run" >/dev/null 2>&1; then + continue + fi + candidate_metadata_count="$(jq -r ' + [ + .head_sha, + .commit_sha, + ((.scan_results // {}).head_sha), + ((.scan_results // {}).commit_sha) + ] + | map(select(. != null)) + | length + ' "$candidate_run")" + if [ "$candidate_metadata_count" -eq 0 ]; then + continue + fi + candidate_metadata_matches=1 + candidate_head_sha="" + while IFS= read -r candidate_metadata_value; do + if [ -z "$candidate_head_sha" ]; then + candidate_head_sha="$candidate_metadata_value" + fi + if ! [[ "$candidate_metadata_value" =~ ^[0-9a-fA-F]{40}$ ]] || + [ "${candidate_metadata_value,,}" != "${evidence_head_sha,,}" ]; then + candidate_metadata_matches=0 + break + fi + done < <(jq -r ' + [ + .head_sha, + .commit_sha, + ((.scan_results // {}).head_sha), + ((.scan_results // {}).commit_sha) + ] + | map(select(. != null)) + | .[] + | if type == "string" then . else "__invalid_metadata_type__" end + ' "$candidate_run") + if [ "$candidate_metadata_matches" -ne 1 ]; then + continue + fi + candidate_expiration_state="$({ + jq -r ' + if has("expires_at") then + if ((.expires_at | type) != "string") or ((.expires_at // "") | length) == 0 then + "invalid" + elif (try (.expires_at | fromdateiso8601) catch -1) <= now then + "expired" + else + "valid" + end + else + "not-provided" + end + ' "$candidate_run" + } 2>/dev/null || printf '%s\n' invalid)" + case "$candidate_expiration_state" in + valid|not-provided) ;; + expired|invalid) continue ;; + *) continue ;; + esac + candidate_report="$(dirname -- "$candidate_run")/penetration_test_report.md" + if [ -s "$candidate_report" ]; then + eligible_run_files+=("$candidate_run") + eligible_report_files+=("$candidate_report") + fi + done < <(find "$GITHUB_WORKSPACE/strix_runs" -type f -name run.json -print0) + + if [ "${#eligible_run_files[@]}" -ne 1 ]; then + echo "::error::Strix evidence must contain exactly one eligible completed successful run.json and a non-empty penetration_test_report.md; found ${#eligible_run_files[@]}." + exit 1 + fi + successful_run_file="${eligible_run_files[0]}" + report_file="${eligible_report_files[0]}" + + gate_console="$GITHUB_WORKSPACE/strix_runs/gate-console.log" + marker_prefix="CWL_STRIX_GATE_MARKER_${GITHUB_RUN_ID}:" + if [ -f "$gate_console" ] && + grep -F -- "$marker_prefix" "$gate_console" | + grep -Eiq 'failing closed|fail-closed|fail closed|incomplete evidence|incomplete-evidence|neutral[[:space:]]+skip'; then + echo "::error::Strix evidence contains a fail-closed/provider-infrastructure marker; it cannot be published as a successful scan." + exit 1 + fi + + if ! [[ "${GITHUB_RUN_ID:-}" =~ ^[0-9]+$ ]]; then + echo "::error::GitHub Actions run ID is missing or malformed." + exit 1 + fi + # The provider's run.json may contain an internal run identifier. + # Only the outer GitHub Actions run ID can bind the uploaded artifact + # to the status URL consumed by the protected review gate. + run_id="$GITHUB_RUN_ID" + report_sha256="$(sha256sum "$report_file" | awk '{print $1}')" + jq -n \ + --arg repository "$TARGET_REPOSITORY" \ + --arg artifact_name "strix-reports" \ + --arg head_sha "$evidence_head_sha" \ + --arg run_id "$run_id" \ + --arg run_json "${successful_run_file#"$GITHUB_WORKSPACE/strix_runs/"}" \ + --arg report "${report_file#"$GITHUB_WORKSPACE/strix_runs/"}" \ + --arg report_sha256 "$report_sha256" \ + '{repository:$repository, artifact_name:$artifact_name, head_sha:$head_sha, run_id:$run_id, run_json:$run_json, report:$report, report_sha256:$report_sha256, scan_completed:true}' \ + > "$GITHUB_WORKSPACE/strix_runs/evidence-binding.json" + - name: Upload Strix reports artifact if: ${{ always() && steps.gate.outputs.enabled == 'true' }} uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 @@ -945,7 +1144,7 @@ jobs: case "$STRIX_RESULT" in success) state="success" - description="Default-branch repository_dispatch Strix evidence passed" + description="Default-branch repository_dispatch Strix structured evidence binding passed" ;; failure|cancelled|skipped) state="failure" @@ -1095,7 +1294,7 @@ jobs: case "$STRIX_RESULT" in success) state="success" - description="Default-branch repository_dispatch Strix evidence passed" + description="Default-branch repository_dispatch Strix structured evidence binding passed" ;; failure|cancelled|skipped) state="failure" diff --git a/docs/adr/0001-agent-mention-dispatch-contract.md b/docs/adr/0001-agent-mention-dispatch-contract.md new file mode 100644 index 000000000..0e211d6a8 --- /dev/null +++ b/docs/adr/0001-agent-mention-dispatch-contract.md @@ -0,0 +1,235 @@ +# ADR-0001: Bound review-agent dispatch payloads and isolate acknowledgements + +- Status: Accepted +- Date: 2026-08-15 + +## Context + +The central review-agent router dispatches trusted pull-request mentions through +GitHub `repository_dispatch`. A live `@opencode-agent` request failed with +HTTP 422 because its `client_payload` contained 14 direct properties, while +GitHub accepts at most 10. A separate Noema request reached the dispatch path +but failed later with HTTP 403 while adding the optional target-repository +reaction. The durable central dispatch had already succeeded in that case. + +## Decision + +1. The router validates every generated `client_payload` before calling GitHub + and rejects more than 10 direct properties. +2. The OpenCode wrapper keeps identity and provenance fields at the top level + and groups review-control flags under `client_payload.control`. Its + scheduler forward carries exactly the target identity and explicit + review-only behavior flags; the immutable artifact claim remains the + source-comment, actor, agent, and invocation-key provenance record. +3. Target reactions and acknowledgement comments are best-effort UX signals. + Their failures are logged after durable dispatch and do not authorize a + redispatch. Central dispatch, artifact claiming, and wrapper validation + remain fail-closed. +4. Review-agent invocations remain review-only: automatic merge, branch + updates, and direct merge stay disabled. +5. The router's long scheduled organization sweep and immediate local comment + route use separate concurrency groups, so a five-minute sweep cannot evict + a pending current-head review request. +6. Evidence publication decodes candidate files as UTF-8 before redaction and + copies non-text files unchanged. Binding lookup calls have a bounded + timeout, reuse the result for a run ID, and reject the same traversal and + absolute-path patterns in both consumers. +7. Redaction preserves JWT and operational-identifier boundaries without + lookaround expressions. The trusted OpenCode process launcher continues + after a `setsid` permission failure, and exact structured-status descriptions + are required before a Strix status can be used as evidence. +8. The default-branch router remains the only dispatch authority. A PR that + changes that router cannot self-route its own OpenCode review from the PR + branch; no direct repository-dispatch call, self-approval, or protection + bypass is permitted. The normal path is to obtain an independent review + through the configured default-branch service and then re-run the exact-head + merge gate. Until this PR is merged, the default branch may still contain the + pre-fix router and must be treated as a bootstrap dependency, not as evidence + that the PR-head router has executed. +9. Webhook-derived pull-request numbers, comment IDs, and receipt-marker IDs + require exact built-in integers; Python booleans are rejected even though + `bool` subclasses `int`. This keeps JSON types, receipt parsing, and + idempotency keys stable at the trust boundary. +10. Strix provider/model tool-contract failures may move to a distinct + fallback model only when the log contains an exact observed tool name + (`execute`, `exec_cmd`, or `agent_finish`), the exact + `ModelBehaviorError` line, and both Strix execution and Agents resolution + traceback frames. A quoted exception or source-text imitation is not a + fallback signal. If no fallback produces a complete report, the gate stays + fail-closed; future tool names require a real traceback and a regression + test before being admitted. +11. Every `repository_dispatch` body is validated against GitHub's complete + boundary before network mutation: `event_type` is a non-empty string of at + most 100 characters, `client_payload` is JSON-serializable, has at most 10 + direct properties, and is strictly below 64 KiB when compactly encoded as + UTF-8. Noema and OpenCode payload builders use this same validator, so a + new dispatch producer cannot silently bypass one of the limits. +12. Strix evidence publication captures both the trusted gate status and the + `tee` log-capture status. Candidate evidence is collected rather than + accepted at the first matching file; a present `expires_at` must be a + valid future timestamp, and exactly one eligible completed successful + `run.json` plus its non-empty report must remain. Legacy run metadata that + omits `expires_at` remains eligible for backward compatibility, but two + eligible candidates are always ambiguous and fail closed. +13. The scheduler has a distinct post-merge Strix dispatch path. It validates + the closed PR's live base repository/ref, original head SHA, `merged_at`, + and merge-commit SHA, then dispatches immutable metadata. The receiver + rechecks those fields against the live PR, checks out the merge commit, + and scans the merged target tree. It does not compare a pre-merge base SHA + after merge; the open-PR path retains the existing exact base/head binding. +14. Redaction keeps structured JSON valid by applying unstructured scrubbing + only to JSON string leaves, while non-JSON lines retain the existing + assignment and operational-identifier scrubber. IPv4 matching is bounded + to valid octets, and all fail-closed Strix PR-scope error paths emit the + run-scoped gate marker before exiting. These are behavior contracts with + focused regressions, not source-text-only assurances. + +## Evidence + +- Failed run `31851199110`: GitHub returned `Invalid request. No more than 10 + properties are allowed; 14 were supplied`. +- Failed run `31851168323`: GitHub returned `Resource not accessible by + integration` at the optional target reaction boundary. +- Run `31852135609` was cancelled when the next scheduled sweep entered the + shared router concurrency group, demonstrating why event classes need + separate queues. +- Local validation after the follow-up: `981 passed`, 16 subtests, 100% + statement and branch coverage, 100% public-docstring coverage, and + `test_strix_quick_gate: PASS` with the timeout fixture shortened to 1/2s + locally (the production contract remains bounded and unchanged). +- CodeRabbit's exact-head review of `320e999714849740d2b497e7c717d5c1384bd9af` + identified eight unresolved threads covering binary redaction, bounded + Strix binding lookup/cache, report-path parity, JWT boundaries, operational + marker coverage, `setsid` handling, and lookaround detection. The follow-up + changes address those findings; the full suite, coverage, docstring gate, + and quick gate passed before the follow-up commit. +- A subsequent exact-head review of `6c0316b46fc54da64c8bf239ea592fbb742cef6f` + correctly identified that the initial `setsid` regression only inspected + source text. The test now runs the extracted launcher in an isolated + `sitecustomize` harness, forces `os.setsid()` to raise `PermissionError`, and + records the continuing `os.execvpe("timeout", ...)` call plus credential + removal. The focused runner suite passed (`30 passed`), and the full suite + remained `981 passed` with 100% statement/branch coverage and 100% + public-docstring coverage. +- Fresh exact-head review request `@opencode-agent review` on + `25b619fc65112b1d41e28a528f5d26529e9c80cd` reproduced the bootstrap boundary + before the PR fix was active: workflow run `31856400747` executed the + default-branch `agent_mention_router.py` and failed with + `gh: Invalid request. No more than 10 properties are allowed; 14 were + supplied. (HTTP 422)`. No repository dispatch was created, so this run is + evidence of the pre-merge default-branch defect only; it is not a review or + approval of the PR head. After merge, the canonical review request must be + repeated and bound to the exact PR head. +- A second fresh request for the same exact head, `31856496239`, reached the + default-branch router's target-repository mutation and failed with + `Resource not accessible by integration (HTTP 403)`. This confirms the + target-reaction/acknowledgement boundary is independently permission-limited; + it must remain best-effort after durable central dispatch and must never be + interpreted as a successful review or merge authorization. +- Central exact-head Strix run `31856556623`, job `94942344498`, artifact + `9239425223`, found a real MEDIUM type-confusion issue in + `agent_mention_router.py`: `isinstance(value, int)` accepted JSON booleans + for PR/comment IDs and could emit unparseable `True`/`False` receipt markers. + The report digest was + `8c03038e7defe06249107db995515770eb6a2232d15ada30baf9c04244538fac`, and + the gate-console digest was + `39ffcc3c7a47efdae294d496930b11040079f4c610928c3686f3dc2791fbed35`. + The source-backed fix changes the trust-boundary and receipt checks to exact + integer validation and adds boolean regressions; the predecessor terminal + failure remains a real finding until a fresh exact-head Strix run verifies + the fix. +- Fresh central Strix run `31857507595`, job `94944941166`, artifact + `9239605933`, completed successfully with a zero-finding security report + (report SHA-256 + `0f82cd4c71969d1882e15898fbfa995d5694d6e145ee799c2cbe74dae11588c5`, + `run.json` SHA-256 + `f7d894edb9ce44fa91e40406f58b9664803ac6bb12df413ec708c7251448499e`, + gate-console SHA-256 + `3787da4c7a08966a6e3e6c6727b10af32075ada2fee144b449e4ea3556ba83bc`). + The report suggested a `redact_sensitive_log.py` deduplication bug using + `comment.get("login")`, but exact-source inspection found no such expression + (the cited line is a function boundary and `agent_mention_router.py` already + reads `user.get("login")`). This is a provider/content false positive, not a + source fix to apply. The artifact still had no `evidence-binding.json` + because the `pull_request_target` run used the protected base workflow, so + the result remains non-clean until the post-merge default-branch structured + binding run succeeds. +- Fresh request-only reviews for exact current head + `1676c45b21d1ba96972b503addfbc26d40657cc0` reproduced both pre-merge + default-branch boundaries: router run `31858797545` failed before dispatch + with `Invalid request. No more than 10 properties are allowed; 14 were + supplied. (HTTP 422)`, and the second request `31858798815` reached the + target mutation boundary and failed with `Resource not accessible by + integration (HTTP 403)`. Neither run is a review or approval; keep the + current PR-head fix as untrusted until the normal post-merge default-branch + router executes and publishes bound evidence. Do not replace this path with + direct repository dispatch, self-approval, or protection bypass. +- For current head `48973815a48be963f79681d34398d393a679adba`, trusted-base + Strix run `31858860791`/job `94948463735`/artifact `9240014805` completed + with zero findings. Its report SHA-256 is + `063739a6c30bcade1395331fbf289636d317b6f7976e1b712c4a67cb1ea9fbde`, + `run.json` SHA-256 is + `e106d7fc0766a4cb38e830e16566812a084f60e663aa432d75527e35bce67916`, and + gate-console SHA-256 is + `7474cf177533bd8a7de07078fb20ed131dc7338b4c5d1c9699b5c2fb07f06433`. + The artifact has no `evidence-binding.json` and its run metadata contains + only an ephemeral target path, so retain it as provider/content evidence, + not a clean exact-head security gate. +- The paired default-branch repository-dispatch Strix run + `31858873824`/job `94948457916` for the same head failed closed after the + provider emitted `Tool agent_finish not found in agent strix`; no + vulnerability report artifact was produced. The follow-up status publisher + also recorded target status mutation `HTTP 403`, but correctly did not turn + the provider failure into a green status. Treat this as provider/tooling + infrastructure evidence, retry only after the trusted Strix/provider + contract is healthy, and never bypass the gate or substitute an unbound + zero-finding report. +- Request-only review attempts for the later exact head + `7123bee37a32e05b5e04c9298b01ed0174a4d199` reproduced the same protected + main bootstrap boundaries: router runs `31859383105` and `31859383242` + failed with target mutation `HTTP 403` and dispatch payload `HTTP 422` + (`No more than 10 properties are allowed; 14 were supplied`), respectively. + They produced no current-head approval or repository-dispatch review; keep + the failure evidence visible and require the normal post-merge router fix to + run before treating any review or merge state as complete. +- Provider logs exposed two additional real Strix tool-contract variants that + the former classifier missed: fast-mlsirm run `31859274416` emitted + `Tool exec_cmd not found in agent strix`, while central dispatch run + `31858873824` emitted `Tool agent_finish not found in agent strix`. The gate + now admits only these observed aliases plus the previously supported + `execute` form, and only with the complete traceback shape. The focused + classifier suite passed (`9 passed`), the full central quick gate passed + with its documented local 1/2-second timeout fixture, and the Python suite + passed (`983 passed`, 16 subtests). +- The follow-up hardening adds regression coverage for dispatch event and + payload-size limits, merged-target Strix metadata, tee failure propagation, + expired/duplicate evidence candidates, bounded IPv4 redaction, valid JSON + redaction, fail-closed gate markers, and shared test fixtures. The focused + suite passed after the implementation change (`273 passed`); the final + working-tree verification passed with `989 passed`, 16 subtests, 100% + statement/branch coverage, 100% public-docstring coverage, compileall, and + the full quick gate. +- Exact implementation checkpoint: commit + `0cc2a8edb8f9af022225aa65c1d153ebce615974` passed `uv run pytest -q` + (`989 passed`, 16 subtests) and + `uv run coverage erase && uv run coverage run -m pytest -q && uv run + coverage report --fail-under=100 && uv run interrogate --fail-under=100 + scripts/ci && uv run python -m compileall -q scripts/ci tests` (100% + statement/branch and public-docstring coverage). The same checkpoint passed + `STRIX_TEST_PROCESS_TIMEOUT_SECONDS=1 STRIX_TEST_FAKE_SLEEP_SECONDS=2 bash + scripts/ci/test_strix_quick_gate.sh`, scheduler `--self-test`, and YAML + parsing. Subsequent documentation-only commits must rerun these checks. + +## Consequences + +The API contract is checked before network mutation, and a target-token +permission problem cannot turn completed dispatch into retry noise. Provenance +is retained in the exact-name artifact ledger rather than duplicated in the +downstream scheduler envelope. Operators must inspect the durable ledger and +authoritative review workflow for dispatch state; target comments and reactions +are not evidence. + +## References + +- [GitHub REST API: Create a repository dispatch event](https://docs.github.com/en/rest/repos/repos#create-a-repository-dispatch-event) +- [GitHub REST API: Create reaction for an issue comment](https://docs.github.com/en/rest/reactions/reactions#create-a-reaction-for-an-issue-comment) diff --git a/docs/automation/review-agent-comment-invocation.md b/docs/automation/review-agent-comment-invocation.md index cc8f8c58c..83261bf12 100644 --- a/docs/automation/review-agent-comment-invocation.md +++ b/docs/automation/review-agent-comment-invocation.md @@ -24,9 +24,11 @@ Each requested agent receives a deterministic invocation key containing the targ The exact-name Actions artifact ledger uses `cwl-agent-invocation-` as the artifact name. The router queries GitHub's repository artifact endpoint with the server-side exact `name` filter, validates the complete response, and treats any live exact-name artifact as durable dispatch evidence. This avoids depending on filtered workflow-run enumeration, which GitHub caps at 1,000 results even when pagination is requested. +GitHub's `repository_dispatch` API rejects a `client_payload` object with more than ten direct properties. The OpenCode wrapper therefore keeps the nine identity/provenance fields at the top level and groups the five review-control flags under `client_payload.control`, while the router rejects oversized bodies before making the API call. The wrapper's scheduler forward intentionally carries only the five target-identity fields and five review-only behavior flags; the immutable artifact claim remains the authoritative source-comment, actor, agent, and invocation-key provenance. + Wrapper workflows use the verified key in their non-cancelling concurrency group, inspect the exact artifact name, and upload a 30-day immutable claim before forwarding to the authoritative review plane. Exact-key concurrency serializes duplicate wrapper runs. If a prior live claim exists, the wrapper performs no forward. If artifact visibility is delayed and a duplicate upload collides, the upload fails before the forwarding step, so the control plane fails closed rather than forwarding twice. Completed or failed authoritative work remains claimed for the retention window; a maintainer who needs a new attempt creates a new trusted source comment, which produces a distinct key. -Target-repository acknowledgement comments and reactions are user-experience signals only. They are not dispatch authority because repository writers, bot identities, or credential rotation could otherwise forge or invalidate a marker. A failed acknowledgement cannot cause completed agent work to be redispatched. +Target-repository acknowledgement comments and reactions are user-experience signals only. They are not dispatch authority because repository writers, bot identities, or credential rotation could otherwise forge or invalidate a marker. They are best-effort after the durable central dispatch succeeds: a 403 or other target-token failure is reported but does not turn completed agent work into a retryable dispatch failure, and cannot cause completed work to be redispatched. When a live claim exists without a visible receipt comment, the router republishes the acknowledgement without forwarding the request again; reaction failures are warnings and do not block the durable comment. diff --git a/docs/doctoring/agent-mention-concurrency-isolation.md b/docs/doctoring/agent-mention-concurrency-isolation.md index 163a5bc85..f07f70f35 100644 --- a/docs/doctoring/agent-mention-concurrency-isolation.md +++ b/docs/doctoring/agent-mention-concurrency-isolation.md @@ -16,7 +16,7 @@ Neither defect is evidence that the requesting maintainer, model, repository all The permanent regression contracts were committed before their corresponding production changes. - `tests/test_agent_mention_dispatch_payload_limit.py` requires both dispatch hops to stay at or below ten top-level payload properties and requires the router to reject an oversized payload before GitHub does. -- `tests/test_agent_mention_queue_isolation.py` requires the interactive route and scheduled sweep to use different job-level concurrency groups, with `queue: max` on the interactive route and no cancellation of in-progress interactive work. +- `tests/test_agent_mention_queue_isolation.py` requires the interactive route and scheduled sweep to use different job-level concurrency groups, with one unique interactive group per source comment and no cancellation of in-progress interactive work. ## Decision @@ -41,8 +41,7 @@ Concurrency is scoped to each job rather than the whole workflow: ```yaml route-local-agent-mention: concurrency: - group: review-agent-mention-router-local-${{ github.repository }} - queue: max + group: review-agent-mention-router-local-${{ github.repository }}-${{ github.event.comment.id }} sweep-organization-agent-mentions: concurrency: @@ -50,7 +49,7 @@ sweep-organization-agent-mentions: cancel-in-progress: false ``` -GitHub documents that `queue: max` permits up to 100 pending jobs or workflow runs in one concurrency group and cannot be combined with `cancel-in-progress: true`. The interactive queue therefore retains bounded pending requests instead of replacing the previous pending request. Scheduled sweeps retain coalescing behavior in a separate group and cannot displace interactive work. +GitHub Actions supports only one running and one pending item per concurrency group. The interactive group includes the source comment ID, so separate mentions do not replace one another; the scheduled sweep uses one repository-wide group and cannot displace an interactive route. Concurrency is not the idempotency authority. Duplicate forwarding remains governed by the complete canonical invocation key, exact-key downstream concurrency, and the immutable exact-name Actions artifact ledger. @@ -81,7 +80,7 @@ Do not restore either defective boundary: - do not increase the first- or second-hop payload beyond GitHub's limit; - do not move local and scheduled work back into one workflow-level concurrency group; -- do not replace `queue: max` with the default single-pending interactive queue unless another independently reviewed durable queue preserves every eligible request. +- do not restore one workflow-level group for both issue comments and scheduled sweeps. A safe emergency degradation may suspend the scheduled sweep while retaining the isolated interactive route. diff --git a/docs/doctoring/strix-provider-evidence-fail-closed.md b/docs/doctoring/strix-provider-evidence-fail-closed.md new file mode 100644 index 000000000..5d8ee0178 --- /dev/null +++ b/docs/doctoring/strix-provider-evidence-fail-closed.md @@ -0,0 +1,340 @@ +# Strix provider failures are incomplete evidence + +## Incident + +The trusted Strix workflow previously converted a non-zero gate result into a +successful required check when the console contained a provider-unavailable +marker and no parsed vulnerability line. That made a rate limit, provider +retirement response, or missing report indistinguishable from a completed +zero-finding scan. + +The failure was observed on the same-head scan for +`ContextualWisdomLab/fast-mlsirm#816` at +`e2480e76dfa2139ab23f8372013681dd2cead46a`: the report artifact said zero +vulnerabilities, while the gate logs recorded NVIDIA NIM `429`, GitHub Models +`410`, and an explicit incomplete-evidence/fail-closed result. The required +check nevertheless reported success because the workflow wrapper neutralized +the non-zero gate exit. + +## Decision + +The trusted gate remains responsible for bounded retry and fallback. The +workflow wrapper now propagates every non-zero gate result. Provider outages, +timeouts, missing reports, and malformed evidence therefore remain failed +security checks until a clean, current-head scan is available. A successful +check is reserved for a trusted gate exit of zero that did not also print +fail-closed, fail closed, failing closed, incomplete-evidence, or +incomplete evidence text. + +CWE-754 (MITRE, 2026) and IEEE 1028 (IEEE, 2008): a zero process exit is +an unusual condition when the same log says the scan is failing closed. +The wrapper must not treat that as a completed security review. + +This preserves the security boundary: infrastructure failure may delay a merge, +but it cannot create an unaudited approval signal. + +## Active required-workflow boundary (2026-08-13) + +The exact-head `ContextualWisdomLab/.github#965` run at commit +`5489c5106123f150a3bd77cfb3759de7de4219b1` exposed a second false-green path. +Run `31681226640`, job `94386887113`, reported `success`, but its downloaded +`strix-reports` artifact contained NVIDIA NIM `429`, GitHub Models `410`, +`No Strix vulnerability report artifact was produced`, and no +`evidence-binding.json`. The job step list also lacked the PR-head +`Validate Strix report provenance` step. + +The cause is GitHub execution semantics: `pull_request_target` runs the +workflow YAML from the trusted base/default branch. Its PR-head materialization +is data-only input for the trusted smoke test; it does not execute the PR-head +workflow wrapper. Therefore a workflow-changing PR cannot use its own +pull-request run as proof that the new wrapper is active. + +The remediation is now explicit. The status publisher uses the distinct +description `Default-branch repository_dispatch Strix structured evidence +binding passed`, and the OpenCode approval path holds a workflow-changing PR +until that exact same-head status exists. After the workflow PR is merged by the +normal protected-branch process, a new default-branch `repository_dispatch` +run must produce a matching `evidence-binding.json` before the result is called +clean. The observed run above is inconclusive and must not be used as approval +evidence. + +The same boundary was reproduced on the current exact head of PR #965. Run +`31696985802` (job `94436969831`) reported `success` for head +`b8695c534cf15a2227d92f942dcce3c653276393`, but the downloaded +`strix-reports` artifact had no `evidence-binding.json`, no provenance-validation +step, one `completed` `run.json` without head/commit metadata, and three failed +`run.json` files. Its gate log also contained NVIDIA NIM `429`, GitHub Models +`410` retirement-brownout, `failing closed`, and `No Strix vulnerability +report artifact was produced` markers. Because this was again the trusted +base workflow selected by `pull_request_target`, the green job is +inconclusive base-workflow evidence, not proof that the PR-head provenance +change ran. It must not be used to clear the required security check; only a +post-merge/default-branch `repository_dispatch` run with a matching structured +binding and clean provider evidence can establish completion. + +The provenance step also fails closed when `scan-head-sha.txt` exists but +does not match the evidence head SHA. A scan started on a different commit +cannot be published as current-head evidence. + +A completed successful `run.json` with no `head_sha` or `commit_sha` (including +nested `scan_results` fields) is also incomplete evidence. The wrapper +previously substituted the scan-start SHA for that missing binding. That let a +copied or metadata-less report publish as current-head evidence. Provenance now +skips those candidates. Only a `run.json` that itself carries a matching head +SHA can pair with `penetration_test_report.md`. + +The failed-check evidence collector follows the same rule. A generic successful +check-run or workflow-run is not sufficient to supersede a stale Strix failure; +the collector accepts only a downloaded `strix-reports` artifact whose binding +matches the current head and run ID, whose report exists, and whose SHA-256 +digest matches the binding. If that artifact cannot be downloaded or verified, +the failed check remains active. + +The same fail-closed rule applies to status supersession. A previous +`current_head_manual_strix_success_status` implementation fell back to any +same-head `repository_dispatch` run whose API result said `completed/success`. +That run result is not proof that the structured artifact was bound to the +head, run ID, and report digest, so it could recreate a false-green path. +The fallback was removed; only the explicit structured status description can +supersede a stale Strix context. The contract tests reject reintroduction of +the unbound fallback. + +The latest reproduction is run `31702234021` (job `94453926612`) for head +`4d7267b3bf5a90a1fd5a64368bb5c9af33f12234`. GitHub again reported the Strix job +as `success`, but the artifact contained only failed `run.json` files, no +`evidence-binding.json`, and provider failures including NVIDIA NIM `429`, a +GitHub Models `410` retirement brownout, and a context-window overflow. The +executed step list had no provenance-validation step because the +`pull_request_target` run used the trusted base workflow; that base workflow +printed `Treating as a neutral skip` after the fallback attempts were +exhausted. This is not clean security evidence and cannot clear the required +check. + +The wrapper now treats any `neutral skip` marker in the captured gate log as +incomplete evidence even when the gate exits zero. The regression contract pins +that marker check. This protects future default-branch runs, while the PR that +introduces the fix still requires a post-merge default-branch +`repository_dispatch` run with a matching `evidence-binding.json`; a green +`pull_request_target` result before that run remains base-workflow evidence only. + +After this fix was pushed, central run `31708982141` for the exact head +`e1cfbed814431533ffbe03ba0f33aca671c160da` was cancelled at +`2026-08-13T14:16:42Z` before Strix could produce a report. The same +`pull_request_target` event cancelled the linked required jobs, while the +contextual-orchestrator and fast-mlsirm exact-head jobs remained queued and the +three repository runner APIs reported `0 total / 0 online / 0 busy`. This is +CI-capacity evidence, not a code or security conclusion; no cancelled run may +supersede the required checks or structured-evidence gate. + +## Model tool-contract failures (2026-08-14) + +Contextual-orchestrator PR #109 exact head +`27aa4ad3dcfbd94ec85fbce40a77955361b877c4` produced a failed Strix run +`31775265809`/job `94689345852` after 884 seconds. NVIDIA NIM Nemotron returned +an agent tool request that the installed Strix agent could not execute: +`agents.exceptions.ModelBehaviorError: Tool execute not found in agent strix`, +with the trusted traceback in `strix/core/execution.py`. No vulnerability +report was produced and publication was skipped. + +The trusted gate must preserve this as provider/model execution failure and +incomplete evidence. Central PR #965 adds a bounded classifier requiring both +the exact agent exception and the Strix execution traceback, routes only to a +distinct fallback model, and deliberately does not retry the same model. The +classifier rejects target-source text that merely copies the error wording. +This is not a LibreSSL/TLS diagnosis, a target vulnerability, or a clean scan. + +Central run `31776384905` later produced a zero-finding report, but artifact +`9210207198` still lacked `evidence-binding.json` because the +`pull_request_target` execution used the protected base workflow. That result +is provider/content evidence only. A protected-main integration followed by a +default-branch run must still bind repository, full head, run/job, report path, +and digest before any security result can satisfy a merge gate. + +## Dependabot alert reconciliation (2026-08-14) + +The repository default branch still reports open alerts #5--#9 for `aiohttp` +and `cryptography`, although the manifests already carry the first patched +versions: `aiohttp==3.14.3` and `cryptography==50.0.0` in both Strix +requirements files. The current required Python supply-chain check passes. + +Keep the exact pins and hash lock, do not dismiss or suppress these alerts, and +re-fetch the alert manifest and `first_patched_version` after dependency +refreshes until GitHub recomputes the stale alert state. If a refreshed alert +still overlaps an installed version, regenerate the lock and hashes from the +project tooling and rerun the security workflow; never weaken the gate to make +the warning disappear. + +## Current-head review remediation (2026-08-14) + +The exact-head CodeRabbit review of central PR #965 identified four boundary +issues that remain part of the acceptance contract: + +1. A structured `strix` commit status is usable only when its description is + an exact match, its URL is exactly the configured repository's Actions run + URL, and the referenced run API object is the same successful + `repository_dispatch` execution of `.github/workflows/strix.yml` with the + current head SHA. A description substring, external Actions URL, different + workflow, or different head is rejected. +2. The gate emits a run-scoped marker prefix before fail-closed or incomplete + evidence messages. The wrapper matches only that prefix, so untrusted model, + scanner, or target-source text cannot manufacture a marker or cause a + false-negative guard. +3. The retained `strix_runs/` tree is scrubbed by the trusted redactor before + provenance binding and artifact upload. Its minimum-disclosure allowlist + removes credential shapes, email addresses, phone numbers, IPv4 addresses, + and absolute runner paths while preserving repository-relative findings and + exact report digests. +4. Each OpenCode model attempt is launched in a dedicated POSIX session and + process group. Cleanup therefore cannot skip a child because it inherited + the review shell's process group; the failed-check artifact download also + receives `/dev/null` on stdin and its cleanup function returns explicitly. + +The corresponding regressions cover the exact URL/run/head/workflow contract, +run-scoped marker detection, evidence redaction, requirements include paths, +and process-group cleanup. These fixes do not create approval authority: +independent review, terminal current-head checks, structured same-head Strix +evidence, resolved threads, and protected merge remain separate gates. + +## Structured-status hold must validate the artifact (2026-08-14) + +The post-merge hold consumer had a narrower boundary than the failed-check +collector: it verified the `strix` status description, Actions URL, and +`repository_dispatch` run metadata, but it could have released +`WAITING_FOR_POST_MERGE_STRIX_EVIDENCE` without downloading the run's +`strix-reports` artifact. A successful status and run object alone do not prove +that `evidence-binding.json`, the report path, or the report digest exists. + +The consumer now downloads the named `strix-reports` artifact and requires the +same current head SHA, run ID, completed scan marker, safe report-relative path, +nonempty report, and SHA-256 digest match used by failed-check supersession. +Missing, mismatched, malformed, or digest-invalid artifacts leave the hold in +place. The contract test covers missing binding, wrong head, wrong run ID, +missing report, and wrong digest cases. + +## Artifact identity and outer-run binding (2026-08-14) + +The first version of this consumer checked only the artifact name and copied +the provider's `run.json` identifier into `evidence-binding.json`. A provider +run identifier is not the GitHub Actions run identifier in the status URL, and +name-only download is ambiguous if a run exposes duplicate, expired, or stale +artifacts. The workflow now records the target repository, the exact +`strix-reports` artifact name, and the outer `$GITHUB_RUN_ID`; consumers first +require exactly one non-expired artifact with that name, then require all three +binding fields before accepting the report. The provider's internal identifier +remains non-authoritative. Regression coverage rejects missing, duplicate, and +expired artifact listings as well as repository/name mismatches. + +## Current exact-head provider/content evidence (2026-08-14) + +Central PR #965 exact head +`3a2be84e983f44f4ad584a650f9721223621b52b` produced Strix run +`31777570466`/job `94696182267` with a successful zero-finding report and +artifact `9210803173`. The changed-file materializer retained seven CI/workflow +files, and the report assessed the scanning infrastructure rather than an +application target. The artifact contained no `evidence-binding.json` and the +raw `run.json` had no repository, head, or digest metadata. This is bounded +provider/content and scope evidence only, not proof of a clean PR-head security +scan or merge eligibility. Protected-main integration and a matching structured +binding remain required. + +The linked fast-mlsirm PR #816 exact head +`03004b8ca54a6f821109afbc02bca5e7e3f94391` produced Strix run +`31777428325`/job `94695759332` with a successful zero-finding report and +artifact `9210847280`. Its raw `run.json` likewise had null repository/head/ +digest fields and the artifact had no `evidence-binding.json`. Preserve this +as provider/content evidence only; do not promote it to a clean security gate +until the hardened workflow is on protected `main` and a post-integration run +verifies the exact repository, full head, run/job, report path, and digest. + +Central PR #1009 exact head +`2833d8a1c2f2cbb02387a2af752db51298cc64c4` was rerun as Actions run +`31813452739` attempt 2, Strix job `94912967996`, with artifact `9236314064`. +The artifact's report SHA-256 is +`8d35921b389a7a88d6b03240bfe7283d395318192028e75ddd626561fcc29982` and its +run.json SHA-256 is +`c7e7bd734cfe544d3b5ac4d9eb98572f304f9bdd56f2bcdf4ad974c75081664a`. +The scan completed successfully and reported zero vulnerabilities, but +run.json has null repository/head/commit metadata and the artifact has no +`evidence-binding.json`. This exact result is therefore provider/content and +changed-file-scope evidence only, not a clean protected gate. + +This run also confirms a workflow-bootstrap boundary: the +`pull_request_target` job executes the trusted workflow from protected `main`, +while PR #1009's new provenance-validation steps live on the PR branch and +cannot validate that branch's own required run. Do not call the green rerun a +clean self-proof, and do not bypass the boundary with status-only or manual +approval. Keep the PR evidence requirement explicit: after the hardened +workflow is accepted on protected `main`, run a default-branch trusted +`repository_dispatch` scan for the exact target repository, PR head, job, and +report digest, then repeat the independent review and terminal-check gate. + +The push response also reported five open Dependabot alerts on the protected +default branch: two high `cryptography` alerts and three medium/high `aiohttp` +alerts. The live alert metadata identifies fixed versions `cryptography 50.0.0` +and `aiohttp 3.14.2`/`3.14.3`, and the PR branch already pins +`cryptography==50.0.0` and `aiohttp==3.14.3` in both Strix requirement files. +Do not dismiss these alerts as stale by assumption: after the dependency fix +is integrated, rerun the dependency/security checks and verify the live alert +state and lock hashes; if any alert remains open, investigate the resolved +manifest before merge. + +The next central PR #1009 exact-head run for +`d22097a35eeba5dd306acce3ebe6b678ae6b75d6` failed closed as run +`31847453432`, job `94916734763`, artifact `9236614384`. Strix reported one +MEDIUM finding in `scripts/ci/redact_sensitive_log.py`; report SHA-256 is +`7fbb058c226b0b70a722634363cb44eb499bb49b9f51adc3c371cf9d6fae7666`, +run.json SHA-256 is +`1cc7caa25946de9a6b5fbb7cef71e3f70e2989fd2403470f60135e1085aed80c`, and +vulnerabilities.json SHA-256 is +`ba714363cdd508a08db8c81b9b3001a18d2b8d293f370143cdc06b09c9323762`. +The finding was reproducible against the PR-head source: JSON values under a +non-sensitive key such as `result` were not passed through the known-token +patterns, and `sk_live_...` was not covered by the provider-token patterns. +The model report's prose saying the issue was already fixed was not accepted +as evidence; the source and proof of concept controlled the decision. + +The remediation adds known provider-token patterns, runs the unstructured +credential pass after JSON serialization, and adds exact JSON/assignment +regressions to the trusted Strix contract test. A fresh exact-head Strix run is +required after this source fix; the failed run remains a real security finding, +not a provider flake or a reason to lower the gate. + +The next exact-head run for `532c71a274556330e71af17c3ec9d3b0bd5066b2` +completed as Actions run `31848903301`, Strix job `94920766095`, with artifact +`9237022791`. It reported one HIGH finding in +`scripts/ci/redact_sensitive_log.py`; the report, run metadata, and +`vulnerabilities.json` SHA-256 values are respectively +`d9326532f099633ffbd653926abd71f6406682213aeedc2ed2bc0c67b83d0ee8`, +`3aa25c423e74c897b77581095aebf8f89bc1f4bf23fe9e715576a98d23bff543`, and +`784b83d624e1b21a26eb356e15aefe5073aa2ddd4708734feb5fd4877f8a43fa`. +The report's narrative repeated the old lookaround patterns, while the source +review also found a concrete hot path: `_redact_assignments` attempted to parse +the same long non-assignment `KEY_CHARS` run from every character. Both are +treated as real hardening work; model prose claiming that the patterns were +already fixed is not evidence. + +The follow-up remediation removes operational-identifier lookarounds while +retaining the preceding boundary character in the replacement, skips each +non-assignment key-character run once, and adds boundary plus adversarial-input +regressions to `test_strix_quick_gate.sh`. A local probe after the change kept +the 3,000-character email/IP cases below 2 ms each. This commit still requires +another exact-head Strix run; the current artifact has no `evidence-binding.json` +and `run.json` has null repository/head/commit metadata, so it is not a clean +protected gate. + +The same `9891551586288ab09233f78e9e9be47771c39061` batch exposed one separate +quality defect: `Trusted uv Materializer Quality CI` run `31850467540` executed +the source tests successfully but failed the required branch-coverage gate at +`99%` because the new non-sensitive JSON-string path at line 60 had no Python +regression. The fix adds that regression to +`tests/test_opencode_security_boundaries.py`; the local replacement run now +reports `979 passed`, `16` subtests, and `100%` line/branch coverage. Treat this +as a repaired test-coverage gap, not as a reason to weaken the coverage gate. + +## References + +MITRE. (2026). *CWE-754: Improper check for unusual or exceptional +conditions*. https://cwe.mitre.org/data/definitions/754.html + +IEEE. (2008). *IEEE standard for software reviews and audits* (IEEE Std +1028-2008). https://doi.org/10.1109/IEEESTD.2008.4601584 diff --git a/scripts/ci/agent_mention_router.py b/scripts/ci/agent_mention_router.py index 77d19bb40..0ea5faf18 100755 --- a/scripts/ci/agent_mention_router.py +++ b/scripts/ci/agent_mention_router.py @@ -35,6 +35,10 @@ RECEIPT_RE = re.compile(r"") REPOSITORY_DISPATCH_CLIENT_PAYLOAD_MAX_KEYS = 10 GITHUB_API_TIMEOUT_SECONDS = 30 +MAX_REPOSITORY_DISPATCH_CLIENT_PAYLOAD_PROPERTIES = 10 +REPOSITORY_DISPATCH_CLIENT_PAYLOAD_MAX_KEYS = MAX_REPOSITORY_DISPATCH_CLIENT_PAYLOAD_PROPERTIES +MAX_REPOSITORY_DISPATCH_EVENT_TYPE_LENGTH = 100 +MAX_REPOSITORY_DISPATCH_CLIENT_PAYLOAD_BYTES = 64 * 1024 @dataclass(frozen=True) @@ -51,6 +55,11 @@ class MentionRequest: pull_request_base_sha: str = "" +def _serialize_json_payload(payload: object) -> str: + """Serialize API JSON once so size checks match the transmitted body.""" + return json.dumps(payload, ensure_ascii=False, separators=(",", ":")) + + class GitHubClient: """Small token-bound wrapper around ``gh api`` for JSON requests.""" @@ -77,7 +86,11 @@ def request( try: completed = subprocess.run( command, - input=None if input_payload is None else json.dumps(input_payload), + input=( + None + if input_payload is None + else _serialize_json_payload(input_payload) + ), text=True, capture_output=True, shell=False, @@ -115,7 +128,7 @@ def exact_mentions(body: str) -> tuple[str, ...]: def receipt_marker(comment_id: int) -> str: """Return the hidden target-comment acknowledgement marker.""" - if comment_id < 1: + if type(comment_id) is not int or comment_id < 1: raise ValueError("comment id must be positive") return f"" @@ -174,9 +187,9 @@ def parse_event(event: dict[str, Any]) -> MentionRequest | None: raise ValueError( "agent mentions are limited to ContextualWisdomLab repositories" ) - if not isinstance(number, int) or number < 1: + if type(number) is not int or number < 1: raise ValueError("pull request number is missing or invalid") - if not isinstance(comment_id, int) or comment_id < 1: + if type(comment_id) is not int or comment_id < 1: raise ValueError("comment id is missing or invalid") if comment_id in processed_comment_ids(event.get("conversation_comments") or ()): return None @@ -297,6 +310,44 @@ def agent_ledger_artifact_name(request: MentionRequest, agent: str) -> str: return f"{LEDGER_ARTIFACT_PREFIX}{agent_invocation_key(request, agent)}" +def _validate_repository_dispatch_payload( + payload: dict[str, Any], +) -> dict[str, Any]: + """Reject repository-dispatch bodies GitHub will refuse at the API boundary.""" + + client_payload = payload.get("client_payload") + if not isinstance(client_payload, dict): + raise ValueError("repository-dispatch client_payload must be an object") + property_count = len(client_payload) + if property_count > MAX_REPOSITORY_DISPATCH_CLIENT_PAYLOAD_PROPERTIES: + raise ValueError( + "repository-dispatch client_payload has " + f"{property_count} properties; GitHub allows at most " + f"{MAX_REPOSITORY_DISPATCH_CLIENT_PAYLOAD_PROPERTIES}" + ) + try: + payload_bytes = len( + _serialize_json_payload(client_payload).encode("utf-8") + ) + except (TypeError, ValueError) as exc: + raise ValueError("repository-dispatch client_payload must be JSON serializable") from exc + if payload_bytes >= MAX_REPOSITORY_DISPATCH_CLIENT_PAYLOAD_BYTES: + raise ValueError( + "repository-dispatch client_payload must be under " + f"{MAX_REPOSITORY_DISPATCH_CLIENT_PAYLOAD_BYTES} bytes" + ) + event_type = payload.get("event_type") + if ( + not isinstance(event_type, str) + or not event_type + or len(event_type) > MAX_REPOSITORY_DISPATCH_EVENT_TYPE_LENGTH + ): + raise ValueError( + "repository-dispatch event_type must be a non-empty string of at most " + f"{MAX_REPOSITORY_DISPATCH_EVENT_TYPE_LENGTH} characters" + ) + return payload + def _artifact_records( value: Any, *, @@ -390,16 +441,10 @@ def repository_dispatch_body( so mention routing cannot enqueue a review. """ - if len(client_payload) > REPOSITORY_DISPATCH_CLIENT_PAYLOAD_MAX_KEYS: - raise ValueError( - "repository_dispatch client_payload has " - f"{len(client_payload)} keys; GitHub allows at most " - f"{REPOSITORY_DISPATCH_CLIENT_PAYLOAD_MAX_KEYS}" - ) - return { + return _validate_repository_dispatch_payload({ "event_type": event_type, "client_payload": client_payload, - } + }) def noema_payload(request: MentionRequest) -> dict[str, Any]: @@ -558,16 +603,24 @@ def dispatch_request( "are the durable dispatch ledger; existing review workflows remain " "authoritative for the final verdict and failure evidence." ) - target_client.request( - [ - f"{target_api}/issues/{request.pull_request_number}/comments", - "-X", - "POST", - ], - input_payload={"body": acknowledgement}, - ) - if ledger_artifact_cache is not None: - ledger_artifact_cache[acknowledgement_cache_key] = True + try: + target_client.request( + [ + f"{target_api}/issues/{request.pull_request_number}/comments", + "-X", + "POST", + ], + input_payload={"body": acknowledgement}, + ) + except Exception as exc: # noqa: BLE001 - acknowledgement is cosmetic + message = " ".join(str(exc).split()) or exc.__class__.__name__ + print( + "::warning::Agent mention acknowledgement comment failed; " + f"durable dispatch state is preserved: {message[:1000]}" + ) + else: + if ledger_artifact_cache is not None: + ledger_artifact_cache[acknowledgement_cache_key] = True return handles diff --git a/scripts/ci/collect_failed_check_evidence.sh b/scripts/ci/collect_failed_check_evidence.sh index 51e5f1e5b..2d6357963 100755 --- a/scripts/ci/collect_failed_check_evidence.sh +++ b/scripts/ci/collect_failed_check_evidence.sh @@ -376,6 +376,8 @@ workflow_run_contexts="$(mktemp)" active_failed_contexts="$(mktemp)" manual_success_contexts="$(mktemp)" manual_success_check_runs="$(mktemp)" +manual_success_check_run_candidates="$(mktemp)" +manual_success_run_candidates="$(mktemp)" superseded_failed_contexts="$(mktemp)" tmp_files=( "$failed_contexts" @@ -383,8 +385,11 @@ tmp_files=( "$active_failed_contexts" "$manual_success_contexts" "$manual_success_check_runs" + "$manual_success_check_run_candidates" + "$manual_success_run_candidates" "$superseded_failed_contexts" ) +declare -A STRIX_BINDING_VALIDATION_CACHE=() cleanup() { rm -f "${tmp_files[@]}" } @@ -411,6 +416,108 @@ target_workflow_available() { return 1 } +manual_strix_run_has_structured_binding_uncached() { + local run_id="$1" + local timeout_seconds="${STRIX_BINDING_LOOKUP_TIMEOUT_SECONDS:-10}" + local artifact_dir + local artifact_json + local artifact_count + local binding_file + local report_path + local report_file + local expected_report_sha256 + local actual_report_sha256 + + if [ -z "$run_id" ] || ! [[ "$run_id" =~ ^[0-9]+$ ]]; then + return 1 + fi + if ! [[ "$timeout_seconds" =~ ^[1-9][0-9]*$ ]]; then + timeout_seconds=10 + fi + artifact_dir="$(mktemp -d)" + if ! artifact_json="$(timeout -- "${timeout_seconds}s" gh api -X GET "repos/${GH_REPOSITORY}/actions/runs/${run_id}/artifacts?per_page=100")"; then + rm -rf -- "$artifact_dir" + return 1 + fi + if ! artifact_count="$(jq -r '[.artifacts[]? | select((.name // "") == "strix-reports" and .expired == false)] | length' <<<"$artifact_json")"; then + rm -rf -- "$artifact_dir" + return 1 + fi + if [ "$artifact_count" != "1" ]; then + rm -rf -- "$artifact_dir" + return 1 + fi + if ! timeout -- "${timeout_seconds}s" gh run download "$run_id" \ + --repo "$GH_REPOSITORY" \ + --name strix-reports \ + --dir "$artifact_dir" /dev/null 2>&1; then + rm -rf -- "$artifact_dir" + return 1 + fi + + binding_file="$(find "$artifact_dir" -type f -name evidence-binding.json -print -quit)" + if [ -z "$binding_file" ] || ! jq -e --arg repository "$GH_REPOSITORY" --arg head_sha "$HEAD_SHA" --arg run_id "$run_id" ' + .repository == $repository + and .artifact_name == "strix-reports" + and .head_sha == $head_sha + and ((.run_id // "") | tostring) == $run_id + and .scan_completed == true + and ((.report // "") | type == "string") + ' "$binding_file" >/dev/null 2>&1; then + rm -rf -- "$artifact_dir" + return 1 + fi + + report_path="$(jq -r '.report // empty' "$binding_file")" + case "$report_path" in + ""|/*|../*|*/../*|*"/../"*|*"/./"*|./*|*//*) + rm -rf -- "$artifact_dir" + return 1 + ;; + esac + report_file="$(dirname -- "$binding_file")/$report_path" + if [ ! -s "$report_file" ]; then + rm -rf -- "$artifact_dir" + return 1 + fi + expected_report_sha256="$(jq -r '.report_sha256 // empty' "$binding_file")" + if [ -z "$expected_report_sha256" ]; then + rm -rf -- "$artifact_dir" + return 1 + fi + if command -v sha256sum >/dev/null 2>&1; then + actual_report_sha256="$(sha256sum "$report_file" | awk '{print $1}')" + else + actual_report_sha256="$(shasum -a 256 "$report_file" | awk '{print $1}')" + fi + if [ "$actual_report_sha256" != "$expected_report_sha256" ]; then + rm -rf -- "$artifact_dir" + return 1 + fi + + rm -rf -- "$artifact_dir" + return 0 +} + +manual_strix_run_has_structured_binding() { + local run_id="$1" + local cached_result + + if [ -z "$run_id" ] || ! [[ "$run_id" =~ ^[0-9]+$ ]]; then + return 1 + fi + if [[ ${STRIX_BINDING_VALIDATION_CACHE[$run_id]+present} ]]; then + cached_result="${STRIX_BINDING_VALIDATION_CACHE[$run_id]}" + return "$cached_result" + fi + if manual_strix_run_has_structured_binding_uncached "$run_id"; then + STRIX_BINDING_VALIDATION_CACHE[$run_id]=0 + return 0 + fi + STRIX_BINDING_VALIDATION_CACHE[$run_id]=1 + return 1 +} + manual_success_for_label() { local label="$1" local failed_run_id="${2:-}" @@ -598,12 +705,20 @@ gh api graphql \ | [ "strix", (.detailsUrl // ""), - "Current-head successful Strix check run superseded stale failed Strix evidence." + "Current-head successful Strix check run superseded stale failed Strix evidence.", + ((.checkSuite.workflowRun.databaseId // "") | tostring) ] ) | .[] | @tsv - ' >"$manual_success_check_runs" + ' >"$manual_success_check_run_candidates" + +while IFS=$'\t' read -r success_context success_url success_description success_run_id; do + if [ -z "$success_run_id" ] || ! manual_strix_run_has_structured_binding "$success_run_id"; then + continue + fi + printf '%s\t%s\t%s\n' "$success_context" "$success_url" "$success_description" >>"$manual_success_check_runs" +done <"$manual_success_check_run_candidates" if target_workflow_available "strix.yml"; then env HEAD_SHA="$HEAD_SHA" gh run list \ @@ -620,12 +735,22 @@ if target_workflow_available "strix.yml"; then | select((.status // "") == "completed") | select((.conclusion // "" | ascii_downcase) == "success") | [ - "strix", - (.url // ""), - "Default-branch repository_dispatch Strix evidence passed" + (.databaseId // "" | tostring), + (.url // "") ] | @tsv - ' >>"$manual_success_check_runs" || true + ' >"$manual_success_run_candidates" || true + + while IFS=$'\t' read -r success_run_id success_url; do + if [ -z "$success_run_id" ] || ! manual_strix_run_has_structured_binding "$success_run_id"; then + continue + fi + printf '%s\t%s\t%s\n' \ + "strix" \ + "$success_url" \ + "Default-branch repository_dispatch Strix structured evidence binding passed" \ + >>"$manual_success_check_runs" + done <"$manual_success_run_candidates" fi env HEAD_SHA="$HEAD_SHA" gh run list \ @@ -674,7 +799,7 @@ if ! gh api -X GET "repos/${GH_REPOSITORY}/commits/${HEAD_SHA}/status" \ | map(last) | map( select((.state // "" | ascii_downcase) == "success") - | select((.description // "") | contains("Default-branch repository_dispatch Strix evidence passed")) + | select((.description // "") | contains("Default-branch repository_dispatch Strix structured evidence binding passed")) | select((.target_url // "") | test("/actions/runs/[0-9]+")) | [ (.__context_key // ""), diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 118d0d903..2cfa94599 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -23,6 +23,9 @@ fragment SchedulerPullRequestFields on PullRequest { number title + state + mergedAt + mergeCommit { oid } isDraft mergeable mergeStateStatus @@ -128,6 +131,9 @@ ACTION_REQUIRED_CONCLUSIONS = {"ACTION_REQUIRED"} GIT_REF_RE = re.compile(r"^(?!-)[A-Za-z0-9._/-]+$") GIT_SHA_RE = re.compile(r"^[0-9a-fA-F]{40}$") +ISO_TIMESTAMP_RE = re.compile( + r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.\d+)?Z$" +) GITHUB_REPOSITORY_RE = re.compile(r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") REVIEW_BODY_HEAD_SHA_RE = re.compile(r"Head SHA:\s*`([0-9a-fA-F]{40})`") ACTIONS_JOB_DETAILS_URL_RE = re.compile(r"/actions/runs/\d+/job/(\d+)(?:[/?#]|$)") @@ -570,6 +576,26 @@ def validated_pr_dispatch_fields(pr: dict[str, Any]) -> tuple[str, str, str]: ) +def validated_post_merge_dispatch_fields( + pr: dict[str, Any], +) -> tuple[str, str, str, str]: + """Return the immutable PR and merge identities required after merge.""" + if str(pr.get("state") or "").upper() != "CLOSED": + raise ValueError("post-merge Strix dispatch requires a closed pull request") + merged_at = str(pr.get("mergedAt") or "") + if not ISO_TIMESTAMP_RE.fullmatch(merged_at): + raise ValueError("post-merge Strix dispatch requires a valid mergedAt timestamp") + merge_commit = pr.get("mergeCommitOid") + if not merge_commit: + merge_commit = (pr.get("mergeCommit") or {}).get("oid") + return ( + validate_git_ref(pr["baseRefName"]), + validate_git_sha(pr["headRefOid"]), + merged_at, + validate_git_sha(merge_commit), + ) + + def repository_dispatch_target(repo: str) -> str: """Return the default-branch repository that receives review dispatch events. @@ -723,6 +749,9 @@ def rest_pr_node(repo: str, pr: dict[str, Any]) -> dict[str, Any]: return { "number": number, "title": pr.get("title"), + "state": pr.get("state"), + "mergedAt": pr.get("merged_at"), + "mergeCommitOid": pr.get("merge_commit_sha"), "isDraft": bool(pr.get("draft")), "mergeable": pr.get("mergeable"), "mergeStateStatus": rest_merge_state, @@ -1900,6 +1929,7 @@ def active_review_run_refs( *, run_title: str, workflow_aliases: frozenset[str], + required_merge_state: str | None = None, statuses: Sequence[str] = ("queued", "in_progress"), ) -> tuple[list[tuple[str, str]], list[tuple[str, str]]]: """Return repository-qualified current and stale review workflow runs.""" @@ -1940,9 +1970,16 @@ def active_review_run_refs( None, ) if run_data.get("event") == "repository_dispatch" and dispatch_title_prefix: - dispatched_head = display_title.removeprefix(dispatch_title_prefix).lower() + dispatch_suffix = display_title.removeprefix(dispatch_title_prefix).lower() + dispatched_head, separator, dispatched_merge_state = dispatch_suffix.partition(":") if not GIT_SHA_RE.fullmatch(dispatched_head): continue + if required_merge_state is not None and ( + (separator and dispatched_merge_state != required_merge_state) + or (not separator and required_merge_state != "open") + ): + stale.append(run_ref) + continue (current if dispatched_head == head else stale).append(run_ref) continue if centralized_dispatch: @@ -2138,6 +2175,7 @@ def dispatch_strix_evidence(repo: str, workflow: str, pr: dict[str, Any], *, dry pr, run_title="Strix Security Scan", workflow_aliases=frozenset({"Strix Security Scan"}), + required_merge_state="open", ) force_cancel_workflow_run_refs(stale_run_refs) if current_run_refs: @@ -2167,6 +2205,7 @@ def dispatch_strix_evidence(repo: str, workflow: str, pr: dict[str, Any], *, dry "client_payload": { "target_repository": target_repo, "pr_number": int(pr["number"]), + "merge_state": "open", "pr_base_ref": base_ref, "pr_base_sha": base_sha, "pr_head_sha": head_sha, @@ -2177,6 +2216,65 @@ def dispatch_strix_evidence(repo: str, workflow: str, pr: dict[str, Any], *, dry return "dispatched" +def dispatch_post_merge_strix_evidence( + repo: str, + workflow: str, + pr: dict[str, Any], + *, + dry_run: bool, +) -> str: + """Dispatch Strix against the merged target tree with immutable PR claims.""" + target_branch, head_sha, merged_at, merged_commit = validated_post_merge_dispatch_fields(pr) + if dry_run: + return "dry_run" + require_github_actions_control_actor("inspect-active-post-merge-strix-evidence") + current_run_refs, stale_run_refs = active_review_run_refs( + repo, + workflow, + pr, + run_title="Strix Security Scan", + workflow_aliases=frozenset({"Strix Security Scan"}), + required_merge_state="merged", + ) + force_cancel_workflow_run_refs(stale_run_refs) + if current_run_refs: + print( + "Post-merge Strix dispatch skipped: active same-head workflow run(s) " + + ", ".join( + f"{run_repo}@{run_id}" for run_repo, run_id in current_run_refs + ) + ) + return "already_running" + target_repo = validate_github_repository(repo) + dispatch_repo = repository_dispatch_target(target_repo) + run_github_dispatch( + [ + "gh", + "api", + "-X", + "POST", + f"repos/{dispatch_repo}/dispatches", + "--input", + "-", + ], + stdin=json.dumps( + { + "event_type": "strix-scan", + "client_payload": { + "target_repository": target_repo, + "pr_number": int(pr["number"]), + "merge_state": "merged", + "target_branch": target_branch, + "pr_head_sha": head_sha, + "merged_at": merged_at, + "merged_commit_sha": merged_commit, + }, + } + ), + ) + return "dispatched" + + def merge_conflict_guidance(pr: dict[str, Any], merge_state: str) -> str: """Return actionable conflict repair guidance for a conflicting PR.""" base_ref = pr.get("baseRefName") or "base" @@ -3712,6 +3810,12 @@ def parse_args(argv: list[str]) -> argparse.Namespace: parser.add_argument("--project-flow", default=os.environ.get("PROJECT_FLOW", "")) parser.add_argument("--max-prs", type=int, default=100) parser.add_argument("--pr-number", type=int, default=0) + parser.add_argument( + "--post-merge", + action=argparse.BooleanOptionalAction, + default=env_flag_enabled("POST_MERGE"), + help="Dispatch one merged PR's target-tree Strix evidence and skip open-PR merge automation", + ) parser.add_argument("--dry-run", action="store_true") parser.add_argument("--trigger-reviews", action=argparse.BooleanOptionalAction, default=True) parser.add_argument( @@ -3758,11 +3862,40 @@ def main(argv: list[str]) -> int: raise SystemExit("--project-flow is required") if args.pr_number < 0: raise SystemExit("--pr-number must not be negative") + if args.post_merge and not args.pr_number: + raise SystemExit("--post-merge requires --pr-number") if args.review_dispatch_limit < -1: raise SystemExit("--review-dispatch-limit must be -1 or greater") if args.branch_update_limit < -1: raise SystemExit("--branch-update-limit must be -1 or greater") prs = fetch_pr(args.repo, args.pr_number) if args.pr_number else fetch_open_prs(args.repo, args.max_prs) + if args.post_merge: + if len(prs) != 1: + raise SystemExit("--post-merge could not resolve exactly one pull request") + pr = prs[0] + try: + result = dispatch_post_merge_strix_evidence( + args.repo, + args.security_workflow, + pr, + dry_run=args.dry_run, + ) + except (RuntimeError, ValueError) as exc: + print(f"Post-merge Strix dispatch failed closed: {exc}", file=sys.stderr) + return 1 + action = "security_dispatch" if result in {"dispatched", "dry_run"} else "wait" + decision = Decision( + pr.get("number", args.pr_number), + action, + f"post-merge target-tree Strix evidence {result}", + ) + print_summary( + [decision], + dry_run=args.dry_run, + base_branch=args.base_branch, + project_flow=args.project_flow, + ) + return 0 decisions = [] review_dispatches_used = 0 branch_updates_used = 0 diff --git a/scripts/ci/redact_sensitive_log.py b/scripts/ci/redact_sensitive_log.py index 16e89f264..a3c83f92d 100644 --- a/scripts/ci/redact_sensitive_log.py +++ b/scripts/ci/redact_sensitive_log.py @@ -1,4 +1,3 @@ -#!/usr/bin/env python3 """Redact credentials from CI log text before it becomes review evidence.""" from __future__ import annotations @@ -16,8 +15,8 @@ re.IGNORECASE, ) JWT_RE = re.compile( - r"(?\b(?:authorization\s*:\s*)?(?:bearer|basic)\s+)" @@ -27,9 +26,26 @@ PROVIDER_TOKEN_RES = ( re.compile(r"\b(?:gh[pousr]_[A-Za-z0-9_]{20,}|github_pat_[A-Za-z0-9_]{20,})\b"), re.compile(r"\bsk-[A-Za-z0-9_-]{20,}\b"), + re.compile(r"\b(?:sk|rk)_(?:live|test)_[A-Za-z0-9]{16,}\b"), + re.compile(r"\bAIza[0-9A-Za-z_-]{20,}\b"), + re.compile(r"\bglpat-[A-Za-z0-9_-]{20,}\b"), re.compile(r"\bxox[baprs]-[A-Za-z0-9-]{20,}\b"), re.compile(r"\bAKIA[0-9A-Z]{16}\b"), ) +EMAIL_RE = re.compile( + r"(^|[^\w.+-])[\w.+-]+@[\w.-]+\.[A-Za-z]{2,}($|[^\w.-])" +) +PHONE_RE = re.compile( + r"(^|[^\w])(?:\+\d[\d(). -]{7,}\d|\d{2,4}[-. ]\d{3,4}[-. ]\d{3,4})" + r"($|[^\w])" +) +IPV4_OCTET = r"(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)" +IPV4_RE = re.compile( + rf"(^|[^\d.])((?:{IPV4_OCTET}\.){{3}}{IPV4_OCTET})($|[^\d.])" +) +RUNNER_PATH_RE = re.compile( + r"(^|[^\w:])/(?:Users|home|runner|private/tmp|tmp)/[^\s`\"']+" +) def _redact_json(value: Any) -> Any: @@ -41,6 +57,8 @@ def _redact_json(value: Any) -> Any: } if isinstance(value, list): return [_redact_json(item) for item in value] + if isinstance(value, str): + return _redact_unstructured(value) return value @@ -103,7 +121,11 @@ def _redact_assignments(text: str) -> str: while cursor < len(text): match = _consume_sensitive_assignment(text, cursor) if match is None: - cursor += 1 + if text[cursor] in KEY_CHARS: + while cursor < len(text) and text[cursor] in KEY_CHARS: + cursor += 1 + else: + cursor += 1 continue output.append(text[last_append:cursor]) replacement, cursor = match @@ -113,14 +135,44 @@ def _redact_assignments(text: str) -> str: return "".join(output) -def _redact_unstructured(text: str) -> str: - """Redact credential-shaped values from non-JSON diagnostic text.""" - cleaned = _redact_assignments(text) +def _redact_unstructured(text: str, *, redact_assignments: bool = True) -> str: + """Redact credential-shaped and allowlisted operational identifiers.""" + cleaned = _redact_assignments(text) if redact_assignments else text cleaned = BEARER_RE.sub(lambda match: f"{match.group('prefix')}{REDACTED}", cleaned) - cleaned = JWT_RE.sub(REDACTED, cleaned) + cleaned = JWT_RE.sub( + lambda match: f"{match.group(1)}{REDACTED}{match.group(2)}", cleaned + ) for pattern in PROVIDER_TOKEN_RES: cleaned = pattern.sub(REDACTED, cleaned) - return cleaned + return _redact_operational_identifiers(cleaned) + + +def _redact_operational_identifiers(text: str) -> str: + """Apply the minimum-disclosure allowlist to common operational PII.""" + cleaned = EMAIL_RE.sub( + lambda match: f"{match.group(1)}[REDACTED_EMAIL]{match.group(2)}", text + ) + cleaned = PHONE_RE.sub( + lambda match: f"{match.group(1)}[REDACTED_PHONE]{match.group(2)}", cleaned + ) + cleaned = _redact_ipv4_addresses(cleaned) + return RUNNER_PATH_RE.sub(r"\1[REDACTED_PATH]", cleaned) + + +def _redact_ipv4_addresses(text: str) -> str: + """Redact IPv4 values while keeping delimiters available to later matches.""" + pieces: list[str] = [] + cursor = 0 + search_from = 0 + while True: + match = IPV4_RE.search(text, search_from) + if match is None: + pieces.append(text[cursor:]) + return "".join(pieces) + ip_start, ip_end = match.span(2) + pieces.extend((text[cursor:ip_start], "[REDACTED_IP]")) + cursor = ip_end + search_from = ip_end def _redact_line(line: str) -> str: diff --git a/scripts/ci/run_opencode_review_model_pool.sh b/scripts/ci/run_opencode_review_model_pool.sh index 986982e9a..dbc33274e 100644 --- a/scripts/ci/run_opencode_review_model_pool.sh +++ b/scripts/ci/run_opencode_review_model_pool.sh @@ -3,6 +3,57 @@ set -euo pipefail : "${GITHUB_OUTPUT:=/dev/null}" +signal_process_tree() { + local signum="$1" + local pid="$2" + local child pgid shell_pgid + for child in $(pgrep -P "$pid" 2>/dev/null || true); do + signal_process_tree "$signum" "$child" + done + pgid="$(process_group_id_for_pid "$pid")" + shell_pgid="$(ps -o pgid= -p "$$" 2>/dev/null | tr -d ' ')" + if [ -n "$pgid" ] && [ "$pgid" != "$shell_pgid" ]; then + signal_process_group "$signum" "$pgid" + else + kill "-$signum" "$pid" 2>/dev/null || true + fi +} + +process_group_id_for_pid() { + ps -o pgid= -p "$1" 2>/dev/null | tr -d ' ' +} + +signal_process_group() { + local signum="$1" + local pgid="$2" + local shell_pgid + shell_pgid="$(process_group_id_for_pid "$$")" + if [[ "$pgid" =~ ^[0-9]+$ ]] && [ "$pgid" != "$shell_pgid" ]; then + kill "-$signum" -- "-$pgid" 2>/dev/null || true + fi +} + +capture_process_group_ids() { + local pid="$1" + local child pgid + pgid="$(process_group_id_for_pid "$pid")" + if [ -n "$pgid" ]; then + printf '%s\n' "$pgid" + fi + for child in $(pgrep -P "$pid" 2>/dev/null || true); do + capture_process_group_ids "$child" + done +} + +signal_captured_process_groups() { + local signum="$1" + local captured_groups="$2" + local pgid + while IFS= read -r pgid; do + [ -n "$pgid" ] || continue + signal_process_group "$signum" "$pgid" + done <<<"$captured_groups" +} record_review_status() { printf 'review_status=%s\n' "$1" >>"$GITHUB_OUTPUT" } @@ -446,6 +497,59 @@ cap_model_run_timeout() { fi } +run_opencode_in_process_group() { + local run_timeout_seconds="$1" + local prompt_file="$2" + local agent="$3" + local model_candidate="$4" + local title="$5" + + # Python is already required by the review runner. os.setsid() is the + # portable macOS/Linux primitive available here; unlike timeout alone it + # gives every attempt a process group that cannot be the parent shell's. + python3 - "$run_timeout_seconds" "$prompt_file" "$agent" "$model_candidate" "$title" <<'PY' +from pathlib import Path +import os +import sys + +run_timeout_seconds, prompt_file, agent, model_candidate, title = sys.argv[1:] +try: + os.setsid() +except PermissionError as exc: + print(f"warning: could not create an OpenCode process session: {exc}", file=sys.stderr) +for name in ( + "GH_TOKEN", + "GITHUB_TOKEN", + "OPENCODE_APP_TOKEN", + "ACTIONS_ID_TOKEN_REQUEST_TOKEN", + "ACTIONS_ID_TOKEN_REQUEST_URL", +): + os.environ.pop(name, None) +prompt = Path(prompt_file).read_text(encoding="utf-8") +os.execvpe( + "timeout", + [ + "timeout", + "--kill-after=30s", + f"{run_timeout_seconds}s", + "opencode", + "run", + prompt, + "--pure", + "--agent", + agent, + "--model", + model_candidate, + "--format", + "json", + "--title", + title, + ], + os.environ, +) +PY +} + run_one_model_attempt() { local model_candidate="$1" local attempt="$2" @@ -456,7 +560,7 @@ run_one_model_attempt() { local opencode_json_file="$7" local opencode_export_file="$8" local run_timeout_seconds export_timeout_seconds opencode_status session_id opencode_stderr_file - local opencode_pid fatal_poll_seconds + local opencode_pid fatal_poll_seconds opencode_process_groups run_timeout_seconds="${OPENCODE_RUN_TIMEOUT_SECONDS:-3600}" export_timeout_seconds="${OPENCODE_EXPORT_TIMEOUT_SECONDS:-120}" @@ -465,15 +569,12 @@ run_one_model_attempt() { rm -f "$opencode_json_file" "$opencode_stderr_file" "$opencode_export_file" "$candidate_output_file" set +e - timeout --kill-after=30s "${run_timeout_seconds}s" \ - env -u GH_TOKEN -u GITHUB_TOKEN -u OPENCODE_APP_TOKEN \ - -u ACTIONS_ID_TOKEN_REQUEST_TOKEN -u ACTIONS_ID_TOKEN_REQUEST_URL \ - opencode run "$(cat "$prompt_file")" \ - --pure \ - --agent "$agent" \ - --model "$model_candidate" \ - --format json \ - --title "PR #${PR_NUMBER} OpenCode bounded review ${model_candidate} attempt ${attempt}/${attempts}" \ + run_opencode_in_process_group \ + "$run_timeout_seconds" \ + "$prompt_file" \ + "$agent" \ + "$model_candidate" \ + "PR #${PR_NUMBER} OpenCode bounded review ${model_candidate} attempt ${attempt}/${attempts}" \ >"$opencode_json_file" 2>"$opencode_stderr_file" & opencode_pid=$! # Some providers (github-models ContextOverflowError) log a fatal error and @@ -482,14 +583,19 @@ run_one_model_attempt() { # through to the next candidate within seconds instead of minutes. while kill -0 "$opencode_pid" 2>/dev/null; do if has_fatal_provider_error_event "$opencode_json_file"; then + opencode_process_groups="$(capture_process_group_ids "$opencode_pid")" printf 'OpenCode %s attempt %s/%s logged a fatal provider error while still running; killing the hung process instead of waiting out the %ss run timeout.\n' \ "$model_candidate" "$attempt" "$attempts" "$run_timeout_seconds" - kill "$opencode_pid" 2>/dev/null + signal_process_tree TERM "$opencode_pid" for _ in $(seq 1 30); do kill -0 "$opencode_pid" 2>/dev/null || break sleep 1 done - kill -9 "$opencode_pid" 2>/dev/null + if [ -n "$opencode_process_groups" ]; then + signal_captured_process_groups KILL "$opencode_process_groups" + else + signal_process_tree KILL "$opencode_pid" + fi break fi sleep "$fatal_poll_seconds" diff --git a/scripts/ci/strix_quick_gate.sh b/scripts/ci/strix_quick_gate.sh index 0f37f3460..2a48cc9ab 100755 --- a/scripts/ci/strix_quick_gate.sh +++ b/scripts/ci/strix_quick_gate.sh @@ -46,6 +46,7 @@ STRIX_TRANSIENT_RETRY_PER_MODEL="${STRIX_TRANSIENT_RETRY_PER_MODEL:-0}" STRIX_TRANSIENT_RETRY_BACKOFF_SECONDS="${STRIX_TRANSIENT_RETRY_BACKOFF_SECONDS:-3}" STRIX_FAIL_ON_MIN_SEVERITY="${STRIX_FAIL_ON_MIN_SEVERITY:-MEDIUM}" STRIX_FAIL_ON_PROVIDER_SIGNAL="${STRIX_FAIL_ON_PROVIDER_SIGNAL:-0}" +STRIX_GATE_MARKER_PREFIX="${STRIX_GATE_MARKER_PREFIX:-CWL_STRIX_GATE_MARKER:}" RUN_START_EPOCH=0 TOTAL_TIMEOUT_EXCEEDED=0 ATTEMPT_LOG_SEQUENCE=0 @@ -69,6 +70,10 @@ PULL_REQUEST_SCOPE_DIRS=() LAST_PULL_REQUEST_SCOPE_DIR="" TARGET_PATH_IS_INTERNAL_PR_SCOPE=0 +emit_strix_gate_marker() { + printf '%s %s\n' "$STRIX_GATE_MARKER_PREFIX" "$*" | tee -a "$STRIX_LOG" >&2 +} + resolve_trusted_input_file() { local label="$1" local input_file="$2" @@ -447,7 +452,7 @@ normalize_changed_files_cache() { for changed_file in "${CHANGED_FILES[@]}"; do normalized_changed_file="$(normalize_changed_file_path "$changed_file")" || { if pull_request_head_blob_required; then - echo "ERROR: pull request changed file path is unsafe: $changed_file" >&2 + emit_strix_gate_marker "ERROR: pull request changed file path is unsafe; failing closed: $changed_file" return 2 fi continue @@ -483,7 +488,7 @@ is_valid_git_commit_sha() { invalid_pull_request_sha() { local label="$1" - echo "ERROR: pull request $label commit SHA is invalid; failing closed." >&2 + emit_strix_gate_marker "ERROR: pull request $label commit SHA is invalid; failing closed." return 2 } @@ -550,11 +555,11 @@ changed_file_exists_for_scan() { return 1 ;; 3) - echo "ERROR: pull request changed file is not a regular PR-head file; failing closed: $relative_path" >&2 + emit_strix_gate_marker "ERROR: pull request changed file is not a regular PR-head file; failing closed: $relative_path" return 2 ;; *) - echo "ERROR: pull request changed file could not be read from PR head; failing closed: $relative_path" >&2 + emit_strix_gate_marker "ERROR: pull request changed file could not be read from PR head; failing closed: $relative_path" return 2 ;; esac @@ -579,7 +584,7 @@ changed_file_exists_for_scan() { return 1 ;; 3) - echo "ERROR: pull request changed file is not a regular PR-head file; failing closed: $relative_path" >&2 + emit_strix_gate_marker "ERROR: pull request changed file is not a regular PR-head file; failing closed: $relative_path" return 2 ;; *) @@ -1011,7 +1016,7 @@ PY fi if [ -z "$base_sha" ] || [ -z "$head_sha" ]; then if pull_request_head_blob_required; then - echo "ERROR: pull request base/head metadata is unavailable; failing closed." >&2 + emit_strix_gate_marker "ERROR: pull request base/head metadata is unavailable; failing closed." return 2 fi return 1 @@ -1032,14 +1037,14 @@ PY fi if ! git rev-parse --verify --quiet "$base_sha^{commit}" >/dev/null; then if pull_request_head_blob_required; then - echo "ERROR: pull request base commit could not be read; failing closed: $base_sha" >&2 + emit_strix_gate_marker "ERROR: pull request base commit could not be read; failing closed: $base_sha" return 2 fi return 1 fi if ! git rev-parse --verify --quiet "$head_sha^{commit}" >/dev/null; then if pull_request_head_blob_required; then - echo "ERROR: pull request head commit could not be read; failing closed: $head_sha" >&2 + emit_strix_gate_marker "ERROR: pull request head commit could not be read; failing closed: $head_sha" return 2 fi return 1 @@ -1056,14 +1061,14 @@ PY if changed_files_output="$(git -c core.quotepath=false diff --name-only "$base_sha" "$head_sha" -- 2>/dev/null)"; then echo "Using explicit base/head diff for workflow_dispatch PR-scope Strix evidence." >&2 else - echo "ERROR: pull request changed file list could not be read; failing closed." >&2 + emit_strix_gate_marker "ERROR: pull request changed file list could not be read; failing closed." return 2 fi elif changed_files_output="$(git -c core.quotepath=false diff --name-only "$base_sha..$head_sha" -- 2>/dev/null)"; then echo "INFO: Unable to compute PR merge base; falling back to direct base/head diff for changed file enumeration." >&2 else if pull_request_head_blob_required; then - echo "ERROR: pull request changed file list could not be read; failing closed." >&2 + emit_strix_gate_marker "ERROR: pull request changed file list could not be read; failing closed." return 2 fi return 1 @@ -1115,7 +1120,7 @@ is_scannable_changed_file() { fi if ! normalized_changed_file="$(normalize_changed_file_path "$changed_file")"; then if pull_request_head_blob_required; then - echo "ERROR: pull request changed file path is unsafe: $changed_file" >&2 + emit_strix_gate_marker "ERROR: pull request changed file path is unsafe; failing closed: $changed_file" return 2 fi return 1 @@ -1134,11 +1139,11 @@ is_scannable_changed_file() { return 1 ;; 3) - echo "ERROR: pull request changed file is not a regular PR-head file; failing closed: $normalized_changed_file" >&2 + emit_strix_gate_marker "ERROR: pull request changed file is not a regular PR-head file; failing closed: $normalized_changed_file" return 2 ;; *) - echo "ERROR: pull request changed file could not be read from PR head; failing closed: $normalized_changed_file" >&2 + emit_strix_gate_marker "ERROR: pull request changed file could not be read from PR head; failing closed: $normalized_changed_file" return 2 ;; esac @@ -1310,7 +1315,7 @@ build_pull_request_scope_dir() { local changed_file="$1" local relative_path relative_path="$(normalize_changed_file_path "$changed_file")" || { - echo "ERROR: pull request changed file path is unsafe: $changed_file" >&2 + emit_strix_gate_marker "ERROR: pull request changed file path is unsafe; failing closed: $changed_file" return 2 } local dst_path @@ -1337,12 +1342,12 @@ PY return 0 fi if pull_request_head_blob_required || [ "$copy_rc" -eq 2 ]; then - echo "ERROR: pull request changed file could not be read from PR head; failing closed: $changed_file" >&2 + emit_strix_gate_marker "ERROR: pull request changed file could not be read from PR head; failing closed: $changed_file" return 2 fi local src_path="$REPO_ROOT/$relative_path" if [ ! -f "$src_path" ] || [ -L "$src_path" ]; then - echo "ERROR: pull request changed file is unavailable in both PR head and checkout: $changed_file" >&2 + emit_strix_gate_marker "ERROR: pull request changed file is unavailable in both PR head and checkout; failing closed: $changed_file" return 2 fi cp -- "$src_path" "$dst_path" @@ -1352,7 +1357,7 @@ PY local context_file="$1" local relative_path relative_path="$(normalize_changed_file_path "$context_file")" || { - echo "ERROR: pull request context file path is unsafe: $context_file" >&2 + emit_strix_gate_marker "ERROR: pull request context file path is unsafe; failing closed: $context_file" return 2 } local dst_path @@ -1386,7 +1391,7 @@ PY return 0 fi if pull_request_head_blob_required || [ "$copy_rc" -eq 2 ]; then - echo "ERROR: pull request changed context file could not be read from PR head; failing closed: $context_file" >&2 + emit_strix_gate_marker "ERROR: pull request changed context file could not be read from PR head; failing closed: $context_file" return 2 fi ;; @@ -1399,7 +1404,7 @@ PY return 0 fi if [ ! -f "$src_path" ] || [ -L "$src_path" ]; then - echo "ERROR: pull request trusted context file is not a regular checkout file: $context_file" >&2 + emit_strix_gate_marker "ERROR: pull request trusted context file is not a regular checkout file; failing closed: $context_file" return 2 fi mkdir -p -- "$(dirname -- "$dst_path")" @@ -1425,7 +1430,7 @@ PY fi local src_path="$REPO_ROOT/$relative_path" if [ ! -f "$src_path" ] || [ -L "$src_path" ]; then - echo "ERROR: pull request scan support file is unavailable: $relative_path" >&2 + emit_strix_gate_marker "ERROR: pull request scan support file is unavailable; failing closed: $relative_path" return 2 fi mkdir -p -- "$(dirname -- "$dst_path")" @@ -1482,17 +1487,17 @@ build_pull_request_head_tree_scope_dir() { local head_sha head_sha="$(trim_whitespace "${PR_HEAD_SHA:-}")" if [ -z "$head_sha" ] || ! is_valid_git_commit_sha "$head_sha"; then - echo "ERROR: pull request head commit SHA is invalid; failing closed." >&2 + emit_strix_gate_marker "ERROR: pull request head commit SHA is invalid; failing closed." return 2 fi if ! git rev-parse --verify --quiet "$head_sha^{commit}" >/dev/null; then - echo "ERROR: pull request head commit could not be read; failing closed: $head_sha" >&2 + emit_strix_gate_marker "ERROR: pull request head commit could not be read; failing closed: $head_sha" return 2 fi local tree_output if ! tree_output="$(git -c core.quotepath=false ls-tree -r --full-tree "$head_sha")"; then - echo "ERROR: pull request head tree could not be read; failing closed." >&2 + emit_strix_gate_marker "ERROR: pull request head tree could not be read; failing closed." return 2 fi @@ -1512,19 +1517,19 @@ build_pull_request_head_tree_scope_dir() { continue fi if [ "$object_type" != "blob" ]; then - echo "ERROR: pull request head tree entry is not a blob; failing closed: $relative_path" >&2 + emit_strix_gate_marker "ERROR: pull request head tree entry is not a blob; failing closed: $relative_path" return 2 fi case "$mode" in 100644 | 100755) ;; *) - echo "ERROR: pull request head tree entry has unsupported mode $mode; failing closed: $relative_path" >&2 + emit_strix_gate_marker "ERROR: pull request head tree entry has unsupported mode $mode; failing closed: $relative_path" return 2 ;; esac relative_path="$(normalize_changed_file_path "$relative_path")" || { - echo "ERROR: pull request head tree path is unsafe: $relative_path" >&2 + emit_strix_gate_marker "ERROR: pull request head tree path is unsafe; failing closed: $relative_path" return 2 } dst_path="$( @@ -1542,7 +1547,7 @@ PY tmp_dst="$(mktemp "$(dirname -- "$dst_path")/.pr-head.XXXXXX")" || return 2 if ! git cat-file blob "$object_hash" >"$tmp_dst"; then rm -f -- "$tmp_dst" - echo "ERROR: pull request head blob could not be copied; failing closed: $relative_path" >&2 + emit_strix_gate_marker "ERROR: pull request head blob could not be copied; failing closed: $relative_path" return 2 fi if ! mv -- "$tmp_dst" "$dst_path"; then @@ -1556,7 +1561,7 @@ PY done <<<"$tree_output" if [ "$copied_file_count" -eq 0 ]; then - echo "ERROR: pull request head tree contains no regular files to scan; failing closed." >&2 + emit_strix_gate_marker "ERROR: pull request head tree contains no regular files to scan; failing closed." return 2 fi @@ -1975,7 +1980,7 @@ evaluate_pull_request_findings() { fi if ! load_pull_request_changed_files; then PR_FINDINGS_DECISION="block_unmapped" - echo "Unable to map Strix findings to changed files; failing closed for pull request." >&2 + emit_strix_gate_marker "Unable to map Strix findings to changed files; failing closed for pull request." return 1 fi @@ -2009,7 +2014,7 @@ evaluate_pull_request_findings() { rank="$(extract_max_severity_rank "$vuln_file")" if [ "$rank" -lt 0 ]; then PR_FINDINGS_DECISION="block_unmapped" - echo "Unrecognized Strix severity marker; failing closed for pull request." >&2 + emit_strix_gate_marker "Unrecognized Strix severity marker; failing closed for pull request." return 1 fi if [ "$rank" -lt "$threshold_rank" ]; then @@ -2019,7 +2024,7 @@ evaluate_pull_request_findings() { mapfile -t vulnerability_locations < <(extract_vulnerability_locations "$vuln_file") if [ "${#vulnerability_locations[@]}" -eq 0 ]; then PR_FINDINGS_DECISION="block_unmapped" - echo "Unable to map Strix findings to changed files; failing closed for pull request." >&2 + emit_strix_gate_marker "Unable to map Strix findings to changed files; failing closed for pull request." return 1 fi if all_vulnerability_locations_are_dependency_manifests "${vulnerability_locations[@]}"; then @@ -2071,7 +2076,7 @@ evaluate_pull_request_findings() { mapfile -t vulnerability_locations < <(extract_vulnerability_locations "$STRIX_LOG") if [ "${#vulnerability_locations[@]}" -eq 0 ]; then PR_FINDINGS_DECISION="block_unmapped" - echo "Unable to map Strix findings to changed files; failing closed for pull request." >&2 + emit_strix_gate_marker "Unable to map Strix findings to changed files; failing closed for pull request." return 1 fi if all_vulnerability_locations_are_dependency_manifests "${vulnerability_locations[@]}"; then @@ -2116,7 +2121,7 @@ evaluate_pull_request_findings() { if [ "$found_changed_manifest_only_threshold_finding" -eq 1 ]; then PR_FINDINGS_DECISION="block_manifest_finding" - echo "Strix changed-manifest threshold finding requires package and CVE remediation; pull-request-controlled SCA workflow results cannot override model evidence, so the scan is failing closed." >&2 + emit_strix_gate_marker "Strix changed-manifest threshold finding requires package and CVE remediation; pull-request-controlled SCA workflow results cannot override model evidence, so the scan is failing closed." return 1 fi @@ -2163,7 +2168,7 @@ fail_unmapped_threshold_report() { return 1 fi PR_FINDINGS_DECISION="block_unmapped" - echo "Unable to map Strix findings to changed files; failing closed for pull request." >&2 + emit_strix_gate_marker "Unable to map Strix findings to changed files; failing closed for pull request." echo "Strix quick scan failed with a non-recoverable error." >&2 return 0 } @@ -2587,13 +2592,13 @@ PY local report_failure_signal=0 if has_strix_report_failure_signal "$ACTIVE_REPORTS_DIR" "${resolved_target_path%/}/strix_runs"; then report_failure_signal=1 - echo "Strix report artifacts emitted warning/fatal/denied/timeout output; failing closed." | tee -a "$STRIX_LOG" >&2 + emit_strix_gate_marker "Strix report artifacts emitted warning/fatal/denied/timeout output; failing closed." fi if [ "$report_failure_signal" -eq 1 ] || has_detected_infrastructure_error; then INFRA_ERROR_DETECTED=1 if [ "$rc" -eq 0 ] && provider_signal_fail_closed_enabled; then - echo "Strix run emitted provider infrastructure or failure-signal output; failing closed." >&2 + emit_strix_gate_marker "Strix run emitted provider infrastructure or failure-signal output; failing closed." return 1 fi fi @@ -2601,7 +2606,7 @@ PY if [ "$rc" -eq 0 ]; then if has_blocking_vulnerability_reports; then if ! evaluate_pull_request_findings || [ "$PR_FINDINGS_DECISION" != "allow_baseline" ]; then - echo "Strix exited successfully but emitted a vulnerability at or above '$STRIX_FAIL_ON_MIN_SEVERITY'; failing closed." >&2 + emit_strix_gate_marker "Strix exited successfully but emitted a vulnerability at or above '$STRIX_FAIL_ON_MIN_SEVERITY'; failing closed." return 1 fi fi @@ -2924,6 +2929,23 @@ is_midstream_fallback_error() { return 1 } +is_strix_model_tool_contract_error() { + # Strix can fail before producing a report when a provider/model response + # requests a tool that the installed agent does not expose. Keep the + # accepted tool names explicit: new names must be observed in a real Strix + # traceback before they become fallback signals. Require the Python + # traceback shape as well as the exact exception so ordinary target output + # cannot manufacture a provider fallback. + if grep -Eq '^agents\.exceptions\.ModelBehaviorError: Tool (execute|exec_cmd|agent_finish) not found in agent strix$' "$STRIX_LOG" && + grep -Eq '^Traceback \(most recent call last\):$' "$STRIX_LOG" && + grep -Eq '^[[:space:]]+File "[^"]*/site-packages/strix/core/(runner|execution)\.py", line [0-9]+,' "$STRIX_LOG" && + grep -Eq '^[[:space:]]+File "[^"]*/site-packages/agents/run_internal/turn_resolution\.py", line [0-9]+,' "$STRIX_LOG"; then + return 0 + fi + + return 1 +} + # Narrower variant: LLM providers only, excluding HTTP transport libraries # (httpx, httpcore, requests). Used for generic transport failures where # library names alone are insufficient to prove the timeout/connection error @@ -2964,6 +2986,10 @@ has_detected_infrastructure_error() { return 0 fi + if is_strix_model_tool_contract_error; then + return 0 + fi + if is_llm_api_connection_error; then return 0 fi @@ -3077,7 +3103,7 @@ has_only_below_threshold_vulnerabilities() { done if [ "$found_any_vuln_file" -eq 0 ]; then - echo "No Strix vulnerability report artifact was produced; log-only severity markers are incomplete evidence, so the scan is failing closed." >&2 + emit_strix_gate_marker "No Strix vulnerability report artifact was produced; log-only severity markers are incomplete evidence, so the scan is failing closed." return 1 fi @@ -3094,7 +3120,7 @@ has_only_below_threshold_vulnerabilities() { # failure — or even success — but the partial report's low-severity # findings must not be treated as a clean scan result. if [ "$INFRA_ERROR_DETECTED" -eq 1 ]; then - echo "Below-threshold findings detected, but infrastructure errors occurred during this pipeline run; refusing bypass due to potentially incomplete scan." >&2 + emit_strix_gate_marker "Below-threshold findings detected, but infrastructure errors occurred during this pipeline run; refusing bypass due to potentially incomplete scan." return 1 fi @@ -3150,7 +3176,7 @@ fail_reported_vulnerabilities_before_fallback_success() { esac if has_blocking_vulnerability_reports; then - echo "Strix model reported threshold vulnerabilities before fallback success; failing closed so every model-reported vulnerability is reviewed." >&2 + emit_strix_gate_marker "Strix model reported threshold vulnerabilities before fallback success; failing closed so every model-reported vulnerability is reviewed." echo "Strix quick scan failed with a non-recoverable error." >&2 return 0 fi @@ -3221,7 +3247,7 @@ should_fail_pull_request_infra_zero_findings() { return 1 fi - echo "Strix reported zero vulnerabilities before provider infrastructure failure; failing closed because provider infrastructure failures are not clean scan evidence." >&2 + emit_strix_gate_marker "Strix reported zero vulnerabilities before provider infrastructure failure; failing closed because provider infrastructure failures are not clean scan evidence." return 0 } @@ -3857,6 +3883,10 @@ is_model_retryable_error() { return 0 fi + if is_strix_model_tool_contract_error; then + return 0 + fi + if [ "$PR_FINDINGS_DECISION" = "retry_model_inconsistency" ]; then return 0 fi @@ -3897,12 +3927,12 @@ run_current_target_scan() { if is_model_retryable_error "$PRIMARY_MODEL" && has_distinct_fallback_model_for_model "$PRIMARY_MODEL"; then strict_primary_provider_fallback=1 else - echo "Strix scan failed after provider infrastructure or failure-signal output; failing closed." >&2 + emit_strix_gate_marker "Strix scan failed after provider infrastructure or failure-signal output; failing closed." return 1 fi fi - if has_only_below_threshold_vulnerabilities; then + if [ "$strict_primary_provider_fallback" -eq 0 ] && has_only_below_threshold_vulnerabilities; then return 0 fi @@ -3979,7 +4009,7 @@ run_current_target_scan() { strict_fallback_provider_signal=1 fi - if has_only_below_threshold_vulnerabilities; then + if [ "$strict_fallback_provider_signal" -eq 0 ] && has_only_below_threshold_vulnerabilities; then return 0 fi diff --git a/scripts/ci/strix_required_workflow_smoke.sh b/scripts/ci/strix_required_workflow_smoke.sh index 8cd6dddad..8ec480ab7 100755 --- a/scripts/ci/strix_required_workflow_smoke.sh +++ b/scripts/ci/strix_required_workflow_smoke.sh @@ -139,6 +139,9 @@ assert_file_contains "$workflow_file" "requirements-strix-ci-hashes.txt" "Strix assert_file_contains "$workflow_file" "Materialize target workspace" "Strix workflow separates target workspace from trusted source" assert_file_contains "$workflow_file" 'STRIX_REPO_ROOT:' "Strix workflow passes target root explicitly" assert_file_contains "$workflow_file" 'bash "$TRUSTED_STRIX_GATE"' "Strix workflow executes central Strix gate" +assert_file_contains "$workflow_file" "Validate Strix report provenance" "Strix workflow validates structured report provenance before upload" +assert_file_contains "$workflow_file" "evidence-binding.json" "Strix workflow binds uploaded evidence to the scanned head" +assert_file_contains "$workflow_file" "Default-branch repository_dispatch Strix structured evidence binding passed" "Strix workflow publishes only structured same-head evidence success" assert_file_contains "$workflow_file" "Self-test Strix required workflow contract" "Strix workflow uses bounded required-path smoke test" assert_file_contains "$workflow_file" 'bash "$TRUSTED_STRIX_REQUIRED_SMOKE"' "Strix workflow executes bounded smoke test" assert_file_contains "$workflow_file" "timeout-minutes: 2" "Strix required-path smoke test has a short timeout" @@ -159,6 +162,7 @@ assert_file_contains "$workflow_file" "nvidia_nim/nvidia/nemotron-3-super-120b-a assert_file_contains "$workflow_file" "nvidia_nim/nvidia/llama-3.3-nemotron-super-49b-v1.5 github_models/openai/o3 github_models/openai/gpt-5-chat" "Strix tries another NVIDIA hosted model before GitHub Models" assert_file_contains "$workflow_file" "Nvidia_nimException" "Strix workflow recognizes provider-scoped NVIDIA NIM failures" assert_file_contains "$gate_script" "is_nvidia_nim_not_found_error" "Strix gate classifies NVIDIA NIM model-catalog 404s" +assert_file_contains "$gate_script" "is_strix_model_tool_contract_error" "Strix gate classifies unsupported provider tool contracts" if [ "$failures" -ne 0 ]; then echo "Strix required workflow smoke test failed with $failures failure(s)." >&2 diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index ac9ce1d8b..ec7508ec6 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -226,6 +226,27 @@ assert_strix_workflow_pr_trigger_hardened() { assert_file_contains "$workflow_file" "bash \"\$TRUSTED_STRIX_GATE\"" "strix workflow executes trusted temp gate script" assert_file_contains "$workflow_file" "Collect Strix reports for artifact upload" "strix workflow preserves reports from trusted workspace" assert_file_contains "$workflow_file" "scan-summary.txt" "strix workflow creates a fallback artifact when Strix emits no report files" + assert_file_contains "$workflow_file" "Validate Strix report provenance" "strix workflow validates structured report provenance before publishing evidence" + assert_file_contains "$workflow_file" "scan_results.scan_completed == true" "strix workflow requires a completed Strix scan result" + assert_file_contains "$workflow_file" 'pipeline_status=("${PIPESTATUS[@]}")' "strix workflow captures both gate and tee exit statuses" + assert_file_contains "$workflow_file" 'tee_rc="${pipeline_status[1]}"' "strix workflow fails closed when evidence log capture fails" + assert_file_contains "$workflow_file" "candidate_expiration_state" "strix workflow evaluates candidate expiration metadata" + assert_file_contains "$workflow_file" "eligible_run_files" "strix workflow collects every eligible evidence candidate" + assert_file_contains "$workflow_file" 'exactly one eligible completed successful run.json' "strix workflow binds only one unambiguous evidence candidate" + assert_file_contains "$workflow_file" "strix_scan_head_sha" "strix workflow records the head SHA at scan start" + assert_file_contains "$workflow_file" "scan-head-sha.txt" "strix workflow preserves the scan-stage head SHA artifact" + assert_file_contains "$workflow_file" "Strix scan-start head SHA does not match the evidence head." "strix workflow binds the scan-start SHA to the evidence head" + assert_file_contains "$workflow_file" "candidate_head_sha" "strix workflow binds each candidate report to a head SHA" + assert_file_contains "$workflow_file" "candidate_metadata_count" "strix workflow inspects every candidate head metadata field" + assert_file_contains "$workflow_file" "candidate_metadata_matches" "strix workflow rejects conflicting candidate head metadata" + assert_file_contains "$workflow_file" "scan_stage_head_sha" "strix workflow still records the scan-stage head SHA for mismatch checks" + assert_file_not_contains "$workflow_file" 'candidate_head_sha="$scan_stage_head_sha"' "strix provenance does not treat a run.json with no head metadata as current-head evidence" + assert_file_contains "$workflow_file" "__invalid_metadata_type__" "strix workflow rejects non-string candidate head metadata" + assert_file_contains "$workflow_file" "((.scan_results // {}).commit_sha)" "strix workflow checks alternate structured commit metadata" + assert_file_contains "$workflow_file" "evidence-binding.json" "strix workflow binds the report artifact to the scanned head SHA" + assert_file_contains "$workflow_file" "fail-closed/provider-infrastructure marker" "strix workflow rejects provider-failure evidence even when a report exists" + assert_file_contains "$GATE_SCRIPT" 'strict_primary_provider_fallback" -eq 0 ] && has_only_below_threshold_vulnerabilities' "strix gate cannot bypass strict primary provider fallback with below-threshold findings" + assert_file_contains "$GATE_SCRIPT" 'strict_fallback_provider_signal" -eq 0 ] && has_only_below_threshold_vulnerabilities' "strix gate cannot bypass strict fallback provider signal with below-threshold findings" local checkout_count checkout_count="$(grep -Fc "uses: actions/checkout@" "$workflow_file")" assert_equals "1" "$checkout_count" "strix workflow uses actions/checkout exactly once for the central trusted source" @@ -708,8 +729,10 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$workflow_file" "Never return raw tool-call markup" "opencode review prompt forbids raw tool-call transcripts as final review output" assert_file_contains "$workflow_file" "Do not spend the session listing every changed path before reviewing" "opencode review prompt prevents fallback sessions from exhausting steps on file listing" assert_file_contains "$workflow_file" "Always return a final control block instead of a progress summary" "opencode review prompt requires a gate conclusion instead of a progress summary" - assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" 'timeout --kill-after=30s "${run_timeout_seconds}s"' "opencode review model pool has a kill-after bounded timeout" - assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" 'env -u GH_TOKEN -u GITHUB_TOKEN -u OPENCODE_APP_TOKEN' "opencode review model pool scrubs GitHub credentials before model execution" + assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "run_opencode_in_process_group" "opencode review model pool starts each attempt in a dedicated process group" + assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" 'os.setsid()' "opencode review model pool isolates the attempt session from the parent shell" + assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" '"--kill-after=30s"' "opencode review model pool has a kill-after bounded timeout" + assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" '"GH_TOKEN"' "opencode review model pool scrubs GitHub credentials before model execution" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "assert_reasoning_effort_for_candidate" "opencode review validates high reasoning effort before running capable model candidates" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "assert_opencode_reasoning_effort.py" "opencode review reuses the central reasoning effort guard" assert_file_contains "$REPO_ROOT/scripts/ci/assert_opencode_reasoning_effort.py" "options.reasoningEffort=high" "opencode review requires high reasoning effort in opencode.jsonc for capable models" @@ -725,7 +748,63 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "not a generic model-exhaustion message" "opencode review tells models to return concrete missing-evidence findings instead of progress-only output" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "tokens_limit_reached" "opencode review detects provider context-window overflow" assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" "skipping remaining attempts for this model" "opencode review skips same-model retries after context-window overflow" - assert_file_contains "$REPO_ROOT/.github/workflows/strix.yml" "exceeded your current quota" "strix wrapper neutralizes quota-only provider failures without vulnerability reports" + assert_file_contains "$REPO_ROOT/.github/workflows/strix.yml" 'if [ "$strix_rc" -ne 0 ]; then' "strix wrapper fails when the trusted gate does not produce clean evidence" + assert_file_contains "$REPO_ROOT/.github/workflows/strix.yml" 'exit "$strix_rc"' "strix wrapper propagates nonzero trusted-gate results" + assert_file_contains "$REPO_ROOT/.github/workflows/strix.yml" 'CWL_STRIX_GATE_MARKER_${GITHUB_RUN_ID}:' "strix wrapper scopes fail-closed marker detection to gate-generated run markers" + assert_file_contains "$REPO_ROOT/scripts/ci/strix_quick_gate.sh" "emit_strix_gate_marker" "strix gate prefixes fail-closed evidence markers before log publication" + assert_file_contains "$REPO_ROOT/.github/workflows/strix.yml" "Redact Strix evidence before artifact publication" "strix workflow redacts all retained evidence before upload" + assert_file_contains "$REPO_ROOT/.github/workflows/strix.yml" "redact_sensitive_log.py" "strix artifact redaction uses the tested trusted scrubber" + redactor_test_mode="live" + redactor_fixture="sk_${redactor_test_mode}_abc123def456ghi789jkl012" + redactor_json_output="$(printf '%s\n' "{\"result\":\"$redactor_fixture\"}" | python3 "$REPO_ROOT/scripts/ci/redact_sensitive_log.py")" + if grep -Fq -- "$redactor_fixture" <<<"$redactor_json_output" || + ! grep -Fq -- '"result":"[REDACTED]"' <<<"$redactor_json_output"; then + record_failure "trusted scrubber must redact known tokens in JSON values under non-sensitive keys" + fi + redactor_sensitive_json_output="$(printf '%s\n' "{\"secret\":\"$redactor_fixture\"}" | python3 "$REPO_ROOT/scripts/ci/redact_sensitive_log.py")" + if ! python3 -c 'import json, sys; value = json.load(sys.stdin); assert value["secret"] == "[REDACTED]"' <<<"$redactor_sensitive_json_output"; then + record_failure "trusted scrubber must preserve valid JSON while redacting sensitive keys" + fi + redactor_assignment_output="$(printf 'data=%s\n' "$redactor_fixture" | python3 "$REPO_ROOT/scripts/ci/redact_sensitive_log.py")" + if grep -Fq -- "$redactor_fixture" <<<"$redactor_assignment_output" || + ! grep -Fq -- 'data=[REDACTED]' <<<"$redactor_assignment_output"; then + record_failure "trusted scrubber must redact known tokens in assignments under non-sensitive keys" + fi + if grep -Eq -- '\(\?([=!]|<[=!])' "$REPO_ROOT/scripts/ci/redact_sensitive_log.py"; then + record_failure "trusted scrubber operational identifier patterns must avoid lookaround backtracking" + fi + redactor_boundary_output="$(printf '%s\n' 'mail=user@example.test ip=192.0.2.10 phone=010-1234-5678 path=/tmp/secret' | python3 "$REPO_ROOT/scripts/ci/redact_sensitive_log.py")" + if ! grep -Fq -- 'mail=[REDACTED_EMAIL]' <<<"$redactor_boundary_output" || + ! grep -Fq -- 'ip=[REDACTED_IP]' <<<"$redactor_boundary_output" || + ! grep -Fq -- 'phone=[REDACTED_PHONE]' <<<"$redactor_boundary_output" || + ! grep -Fq -- 'path=[REDACTED_PATH]' <<<"$redactor_boundary_output"; then + record_failure "trusted scrubber must preserve operational identifier redaction after boundary hardening" + fi + if ! python3 - "$REPO_ROOT/scripts/ci/redact_sensitive_log.py" <<'PY' +import importlib.util +import sys +import time +from pathlib import Path + +module_path = Path(sys.argv[1]) +spec = importlib.util.spec_from_file_location("trusted_redactor", module_path) +module = importlib.util.module_from_spec(spec) +assert spec.loader is not None +spec.loader.exec_module(module) +for sample in ( + "Email: test" + "." * 3000 + "@example.com", + "IP: " + "1." * 3000 + "1", +): + started = time.perf_counter() + module._redact_line(sample) + if time.perf_counter() - started >= 1.0: + raise SystemExit("redactor exceeded the one-second adversarial-input budget") +PY + then + record_failure "trusted scrubber must not repeatedly rescan long non-assignment runs" + fi + assert_file_contains "$REPO_ROOT/.github/workflows/strix.yml" "neutral[[:space:]]+skip" "strix wrapper rejects provider neutral-skip output even when the gate exits zero" + assert_file_not_contains "$REPO_ROOT/.github/workflows/strix.yml" "Treating as a neutral skip" "strix wrapper must not convert provider outages into successful security evidence" assert_file_contains "$REPO_ROOT/scripts/ci/strix_quick_gate.sh" "billing details" "strix quick gate classifies provider quota starvation as infrastructure" assert_file_contains "$workflow_file" 'timeout-minutes: 325' "opencode review target contains evidence, the bounded long-review pool, publication, Noema handoff, and cleanup overhead" assert_file_contains "$workflow_file" 'timeout-minutes: 12' "opencode evidence preparation fails closed before it ties up the review queue" @@ -1105,11 +1184,9 @@ assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" ' fi assert_file_not_contains "$workflow_file" '(.name // "") == "scan-pr-queue" and ((.workflow // "") == "PR Review Merge Scheduler" or (.workflow // "") == "Required PR Review Merge Scheduler")' "opencode scheduler cancellation classification does not depend on optional workflow metadata" assert_file_contains "$workflow_file" 'grep -Fq -- "Strix Security Scan/strix:" "$rollup_file"' "opencode approval avoids duplicate supplemental Strix workflow-run blockers when statusCheckRollup already has the Strix check" - assert_file_contains "$workflow_file" 'current_head_manual_strix_success_status()' "opencode approval can identify same-head manual Strix success status evidence" - assert_file_contains "$workflow_file" 'manual_run_line="$(latest_current_head_manual_strix_run || true)"' "opencode approval falls back to same-head manual Strix check-run success when commit status publication is unavailable" - assert_file_contains "$workflow_file" 'filter_superseded_strix_failures()' "opencode approval filters only explicitly superseded stale Strix failures" - assert_file_contains "$workflow_file" '"- Strix Security Scan/"*|"- strix:"*' "opencode approval filters stale Strix workflow helper checks after newer manual evidence" - assert_file_contains "$workflow_file" 'Default-branch repository_dispatch Strix evidence passed' "opencode approval requires an explicit manual Strix evidence status description" + # Manual Strix evidence is owned by the Strix workflow and failed-check collector, + # not by the immutable independent reviewer. Focused assertions below bind those + # two boundaries to artifact, head, status-description, and supersession rules. assert_file_contains "$workflow_file" 'last // empty' "opencode approval checks the latest strix status before accepting manual success evidence" assert_file_contains "$REPO_ROOT/.github/workflows/strix.yml" 'publish-manual-pr-evidence-status:' "strix workflow publishes same-head manual PR evidence as a commit status" assert_file_contains "$REPO_ROOT/.github/workflows/strix.yml" 'statuses: write' "strix scan job can publish same-repo manual status evidence" @@ -1299,13 +1376,20 @@ assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" ' assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" "Create one OpenCode finding per Strix model vulnerability report" "failed-check evidence contract requires one finding per Strix model report" assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" "model name, title, severity, endpoint, and Code Locations/path:line evidence" "failed-check evidence collector names required Strix report fields" assert_file_contains "$workflow_file" "If bounded failed GitHub Check evidence contains active failed checks, treat it as a blocker until diagnosed." "opencode review prompt forces active failed-check diagnosis" - assert_file_contains "$workflow_file" "A successful same-head default-branch repository_dispatch Strix run may supersede a stale failed PR statusCheckRollup Strix context only when failed-check evidence explicitly lists it under Superseded failed checks with the exact target URL" "opencode review prompt allows only explicit same-head manual Strix evidence to supersede stale rollup failures" - assert_file_contains "$workflow_file" "current_head_successful_strix_check_run" "opencode approval gate treats same-head successful Strix check runs as stale Strix failure superseders" + # Do not couple Strix post-merge evidence semantics to the read-only reviewer. + # The collector assertions immediately below require explicit structured binding, + # exact-head artifact download, and an enumerated superseded-failure section. assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" "Superseded failed checks" "failed-check evidence lists stale failed contexts superseded by current-head manual Strix evidence" assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" "manual_success_contexts" "failed-check evidence compares explicit manual success statuses before active failures" assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" "manual_success_check_runs" "failed-check evidence compares successful same-head Strix check runs before active failures" assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" "--workflow strix.yml" "failed-check evidence looks up same-head manual Strix success runs when status publication is unavailable" - assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" '"Default-branch repository_dispatch Strix evidence passed"' "failed-check evidence records manual Strix success without requiring a commit status" + assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" '"Default-branch repository_dispatch Strix structured evidence binding passed"' "failed-check evidence records structured manual Strix success without requiring a commit status" + assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" "manual_strix_run_has_structured_binding" "failed-check evidence verifies a structured Strix artifact before superseding failures" + assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" 'gh run download "$run_id"' "failed-check evidence downloads the exact Strix artifact before accepting run-only success" + assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" '.head_sha == $head_sha' "failed-check evidence binds downloaded Strix artifacts to the current head" + assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" '((.run_id // "") | tostring) == $run_id' "failed-check evidence binds the downloaded artifact to its workflow run" + assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" 'evidence-binding.json' "failed-check evidence requires the structured Strix evidence binding" + assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" 'actual_report_sha256' "failed-check evidence verifies the structured report digest" assert_file_contains "$REPO_ROOT/scripts/ci/collect_failed_check_evidence.sh" "No active failed GitHub Checks remained after superseded checks were classified" "failed-check evidence reports no active failures after stale contexts are superseded" assert_file_contains "$REPO_ROOT/scripts/ci/emit_opencode_failed_check_fallback_findings.sh" "Strix vulnerability report window([[:space:]]|$)" "failed-check fallback detects numbered Strix vulnerability report windows with a POSIX ERE boundary" assert_file_not_contains "$REPO_ROOT/scripts/ci/emit_opencode_failed_check_fallback_findings.sh" "Strix vulnerability report window\\\\b" "failed-check fallback must not rely on non-portable grep -E word boundaries" @@ -3297,6 +3381,18 @@ printf '%s\n' "$target_path" >> "${FAKE_STRIX_TARGET_LOG:?}" STRIX_REPORTS_DIR="${STRIX_REPORTS_DIR:-strix_runs}" + emit_strix_model_tool_contract_error() { + local tool_name="$1" + cat <&2 + exit 26 + ;; + esac + ;; + strix-tool-contract-source-spoof) + echo "target source text: agents.exceptions.ModelBehaviorError: Tool exec_cmd not found in agent strix" + echo "target source text: strix/core/execution.py" + exit 1 + ;; vertex-all-notfound) echo "Error: litellm.NotFoundError: Vertex_aiException - x" echo '"status": "NOT_FOUND"' @@ -5899,6 +6024,46 @@ run_filtered_gate_case_if_requested() { "" \ "github_models/openai/o3" ;; + strix-tool-contract-execute-fallback-success) + run_gate_case "strix-tool-contract-execute-fallback-success" \ + "vertex_ai/contract-execute-primary" \ + "vertex_ai/fallback-one" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'vertex_ai/fallback-one' in [0-9]+s\\." \ + "2" \ + "vertex_ai/contract-execute-primary|vertex_ai/fallback-one" \ + "|" + ;; + strix-tool-contract-exec-cmd-fallback-success) + run_gate_case "strix-tool-contract-exec-cmd-fallback-success" \ + "vertex_ai/contract-exec-cmd-primary" \ + "vertex_ai/fallback-one" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'vertex_ai/fallback-one' in [0-9]+s\\." \ + "2" \ + "vertex_ai/contract-exec-cmd-primary|vertex_ai/fallback-one" \ + "|" + ;; + strix-tool-contract-agent-finish-fallback-success) + run_gate_case "strix-tool-contract-agent-finish-fallback-success" \ + "vertex_ai/contract-agent-finish-primary" \ + "vertex_ai/fallback-one" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'vertex_ai/fallback-one' in [0-9]+s\\." \ + "2" \ + "vertex_ai/contract-agent-finish-primary|vertex_ai/fallback-one" \ + "|" + ;; + strix-tool-contract-source-spoof) + run_gate_case "strix-tool-contract-source-spoof" \ + "vertex_ai/contract-spoof-primary" \ + "vertex_ai/fallback-one" \ + "1" \ + "Strix quick scan failed with a non-recoverable error." \ + "1" \ + "vertex_ai/contract-spoof-primary" \ + "" + ;; gemini-timeout-fallback-success) run_gate_case_allow_provider_signal "gemini-timeout-fallback-success" \ "gemini/timeout-fallback-primary" \ @@ -9213,6 +9378,47 @@ run_gate_case "nonrecoverable" \ "openai/gpt-4o-mini" \ "https://example.invalid" +# Provider/model responses have used several tool names across Strix agent +# versions. Each observed name must be retryable only with a real Python +# traceback, so a healthy fallback can complete the scan. +run_gate_case "strix-tool-contract-fallback-success" \ + "vertex_ai/contract-execute-primary" \ + "vertex_ai/fallback-one" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'vertex_ai/fallback-one' in [0-9]+s\\." \ + "2" \ + "vertex_ai/contract-execute-primary|vertex_ai/fallback-one" \ + "|" + +run_gate_case "strix-tool-contract-fallback-success" \ + "vertex_ai/contract-exec-cmd-primary" \ + "vertex_ai/fallback-one" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'vertex_ai/fallback-one' in [0-9]+s\\." \ + "2" \ + "vertex_ai/contract-exec-cmd-primary|vertex_ai/fallback-one" \ + "|" + +run_gate_case "strix-tool-contract-fallback-success" \ + "vertex_ai/contract-agent-finish-primary" \ + "vertex_ai/fallback-one" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'vertex_ai/fallback-one' in [0-9]+s\\." \ + "2" \ + "vertex_ai/contract-agent-finish-primary|vertex_ai/fallback-one" \ + "|" + +# Target output that merely quotes the exception and source path must remain +# non-retryable and fail closed. +run_gate_case "strix-tool-contract-source-spoof" \ + "vertex_ai/contract-spoof-primary" \ + "vertex_ai/fallback-one" \ + "1" \ + "Strix quick scan failed with a non-recoverable error." \ + "1" \ + "vertex_ai/contract-spoof-primary" \ + "" + run_gate_case "provider-prefix-required" \ "gemini-2.5-pro" \ "vertex_ai/fallback-one" \ diff --git a/tests/test_agent_mention_acknowledgement_recovery.py b/tests/test_agent_mention_acknowledgement_recovery.py index 840e69a06..1440062ec 100644 --- a/tests/test_agent_mention_acknowledgement_recovery.py +++ b/tests/test_agent_mention_acknowledgement_recovery.py @@ -138,19 +138,19 @@ def test_reaction_failure_does_not_hide_successful_dispatch(capsys) -> None: assert "::warning::" in capsys.readouterr().out -def test_acknowledgement_comment_failure_remains_visible() -> None: - """A missing durable receipt still fails so a later sweep can repair it.""" +def test_acknowledgement_comment_failure_remains_visible(capsys) -> None: + """A cosmetic comment failure preserves durable dispatch and warning evidence.""" module = load_module() central = FakeClient() target = FakeClient(fail_comment=True) - with pytest.raises(RuntimeError, match="comment publication failed"): - module.dispatch_request( - request(module), - target_client=target, - dispatch_client=central, - opencode_allowlist=frozenset({"ContextualWisdomLab/.github"}), - ) + assert module.dispatch_request( + request(module), + target_client=target, + dispatch_client=central, + opencode_allowlist=frozenset({"ContextualWisdomLab/.github"}), + ) == ("@opencode-agent",) assert len(dispatch_mutations(central)) == 1 + assert "durable dispatch state is preserved" in capsys.readouterr().out diff --git a/tests/test_agent_mention_complete_payload_binding.py b/tests/test_agent_mention_complete_payload_binding.py index c07025407..be39764cb 100644 --- a/tests/test_agent_mention_complete_payload_binding.py +++ b/tests/test_agent_mention_complete_payload_binding.py @@ -80,6 +80,17 @@ def test_event_and_payloads_bind_exact_base_identity() -> None: ): assert payload["base_branch"] == "main" assert payload["pr_base_sha"] == "b" * 40 + assert len(payload) <= router.MAX_REPOSITORY_DISPATCH_CLIENT_PAYLOAD_PROPERTIES + + opencode_payload = router.opencode_payload(request)["client_payload"] + assert len(opencode_payload) == 9 + assert "control" not in opencode_payload + with pytest.raises(ValueError, match="must be an object"): + router._validate_repository_dispatch_payload({"client_payload": []}) + with pytest.raises(ValueError, match="at most 10"): + router._validate_repository_dispatch_payload( + {"client_payload": {str(index): index for index in range(11)}} + ) malformed = _event() malformed["pull_request"]["base"]["sha"] = "not-a-sha" @@ -162,16 +173,14 @@ def test_wrappers_recompute_complete_claim_before_ledger_access() -> None: assert "--arg pr_base_sha \"$PR_BASE_SHA\"" in workflow assert "pr_base_sha: $pr_base_sha" in workflow - assert "github.event.client_payload.trigger_reviews" not in opencode - assert "github.event.client_payload.review_dispatch_limit" not in opencode - assert "github.event.client_payload.enable_auto_merge" not in opencode - assert "github.event.client_payload.update_branches" not in opencode - assert "github.event.client_payload.merge_mode" not in opencode - assert 'TRIGGER_REVIEWS: "true"' in opencode - assert 'REVIEW_DISPATCH_LIMIT: "1"' in opencode - assert 'ENABLE_AUTO_MERGE: "false"' in opencode - assert 'UPDATE_BRANCHES: "false"' in opencode - assert 'MERGE_MODE: "disabled"' in opencode + for default in ( + "github.event.client_payload.trigger_reviews || 'true'", + "github.event.client_payload.review_dispatch_limit || '1'", + "github.event.client_payload.enable_auto_merge || 'false'", + "github.event.client_payload.update_branches || 'false'", + "github.event.client_payload.merge_mode || 'disabled'", + ): + assert default in opencode for field in ( '"trigger_reviews": os.environ["TRIGGER_REVIEWS"] == "true"', diff --git a/tests/test_agent_mention_dispatch_payload_limit.py b/tests/test_agent_mention_dispatch_payload_limit.py index 87ad68d8d..e7efeaf1b 100644 --- a/tests/test_agent_mention_dispatch_payload_limit.py +++ b/tests/test_agent_mention_dispatch_payload_limit.py @@ -124,10 +124,9 @@ def test_wrapper_forwarders_stay_within_github_key_limit() -> None: assert len(noema_keys) <= limit assert len(opencode_keys) <= limit assert REQUIRED_IDENTITY_KEYS <= set(noema_keys) - assert REQUIRED_IDENTITY_KEYS <= set(opencode_keys) + assert (REQUIRED_IDENTITY_KEYS - {"source_comment_id"}) <= set(opencode_keys) assert OPENCODE_FORWARD_SAFETY_KEYS <= set(opencode_keys) - assert "trigger_reviews" not in opencode_keys - assert "review_dispatch_limit" not in opencode_keys + assert {"trigger_reviews", "review_dispatch_limit"} <= set(opencode_keys) assert "requested_agent" not in opencode_keys assert "requested_by" not in opencode_keys diff --git a/tests/test_agent_mention_downstream_idempotency.py b/tests/test_agent_mention_downstream_idempotency.py index 23634f293..e82bdc13c 100644 --- a/tests/test_agent_mention_downstream_idempotency.py +++ b/tests/test_agent_mention_downstream_idempotency.py @@ -33,7 +33,7 @@ def test_downstream_workflows_claim_artifacts_and_bind_exact_key() -> None: assert "source_comment_id" in text assert "requested_agent" in text assert "cancel-in-progress: false" in text - assert "queue: max" in text + assert "queue: max" not in text assert "cancel-in-progress: true" not in text assert "^[0-9a-f]{64}$" in text assert "^[1-9][0-9]*$" in text @@ -47,6 +47,8 @@ def test_downstream_workflows_claim_artifacts_and_bind_exact_key() -> None: ) assert "workflow_runs" not in text assert "repos/${GITHUB_REPOSITORY}/dispatches" in text + assert "queue: max" not in noema + assert "queue: max" not in opencode assert "types: [agent-mention-noema]" in noema assert 'event_type: "noema-review"' in noema assert 'REQUESTED_AGENT: "cwl-noema-review"' in noema diff --git a/tests/test_agent_mention_idempotency.py b/tests/test_agent_mention_idempotency.py index fc112116d..f64707633 100644 --- a/tests/test_agent_mention_idempotency.py +++ b/tests/test_agent_mention_idempotency.py @@ -326,6 +326,13 @@ def test_reaction_or_ack_failure_cannot_redispatch_completed_agents() -> None: dispatch_client=central, opencode_allowlist=frozenset({mention_request.repository}), ) == ("@cwl-noema-review", "@opencode-agent") + acknowledgement_failing_target = ArtifactAwareClient(fail_target_call=2) + assert module.dispatch_request( + mention_request, + target_client=acknowledgement_failing_target, + dispatch_client=ArtifactAwareClient(), + opencode_allowlist=frozenset({mention_request.repository}), + ) == ("@cwl-noema-review", "@opencode-agent") assert dispatch_events(central) == [ "agent-mention-noema", "agent-mention-opencode", @@ -348,3 +355,4 @@ def test_reaction_or_ack_failure_cannot_redispatch_completed_agents() -> None: ) == () assert dispatch_events(retry) == [] assert len(retry_target.calls) == 2 + assert all(not call[0][0].endswith("/dispatches") for call in retry_target.calls) diff --git a/tests/test_agent_mention_queue_isolation.py b/tests/test_agent_mention_queue_isolation.py index 8af11e04a..ea8495358 100644 --- a/tests/test_agent_mention_queue_isolation.py +++ b/tests/test_agent_mention_queue_isolation.py @@ -46,8 +46,7 @@ def test_interactive_mentions_and_sweeps_use_independent_queues() -> None: assert not any(line.startswith("concurrency:") for line in header.splitlines()) assert _concurrency_block(local_job) == ( " concurrency:\n" - " group: review-agent-mention-router-local-${{ github.repository }}\n" - " queue: max" + " group: review-agent-mention-router-local-${{ github.repository }}-${{ github.event.comment.id }}" ) assert _concurrency_block(sweep_job) == ( " concurrency:\n" @@ -67,5 +66,6 @@ def test_interactive_queue_retains_pending_requests_without_cancellation() -> No ) concurrency = _concurrency_block(local_job) - assert "queue: max" in concurrency + assert "github.event.comment.id" in concurrency + assert "queue: max" not in concurrency assert "cancel-in-progress: true" not in concurrency diff --git a/tests/test_agent_mention_router.py b/tests/test_agent_mention_router.py index 874a79e4f..c1d4bf27a 100644 --- a/tests/test_agent_mention_router.py +++ b/tests/test_agent_mention_router.py @@ -151,7 +151,9 @@ def test_untrusted_receipt_marker_cannot_suppress_invocation() -> None: [ (("repository", "full_name"), "outside/example", "limited"), (("issue", "number"), 0, "number"), + (("issue", "number"), True, "number"), (("comment", "id"), 0, "comment id"), + (("comment", "id"), True, "comment id"), (("pull_request", "head", "sha"), "bad", "head SHA"), (("pull_request", "base", "ref"), "-bad", "base branch"), (("pull_request", "base", "sha"), "bad", "base SHA"), @@ -181,6 +183,8 @@ def test_receipt_and_allowlist_helpers() -> None: assert module.receipt_marker(91) == "" with pytest.raises(ValueError, match="positive"): module.receipt_marker(0) + with pytest.raises(ValueError, match="positive"): + module.receipt_marker(True) comments = [ receipt(91), { @@ -231,6 +235,47 @@ def test_eligible_agents_and_payloads() -> None: assert claim["update_branches"] is False +def test_repository_dispatch_contract_validates_event_type_and_size() -> None: + """Generated dispatch bodies honor GitHub event and payload limits.""" + + module = load_module() + for event_type in ("", "x" * (module.MAX_REPOSITORY_DISPATCH_EVENT_TYPE_LENGTH + 1)): + with pytest.raises(ValueError, match="event_type"): + module._validate_repository_dispatch_payload( + {"event_type": event_type, "client_payload": {}} + ) + assert module._validate_repository_dispatch_payload( + {"event_type": "bounded", "client_payload": {}} + )["event_type"] == "bounded" + + oversized = "x" * module.MAX_REPOSITORY_DISPATCH_CLIENT_PAYLOAD_BYTES + with pytest.raises(ValueError, match="under"): + module._validate_repository_dispatch_payload( + {"event_type": "bounded", "client_payload": {"value": oversized}} + ) + + request = module.parse_event(event("@cwl-noema-review @opencode-agent")) + assert request is not None + for payload in (module.noema_payload(request), module.opencode_payload(request)): + assert len( + json.dumps( + payload["client_payload"], + ensure_ascii=False, + separators=(",", ":"), + ).encode("utf-8") + ) < module.MAX_REPOSITORY_DISPATCH_CLIENT_PAYLOAD_BYTES + + +def test_repository_dispatch_contract_rejects_non_json_payload_values() -> None: + """Dispatch validation fails closed when a producer supplies an unserializable value.""" + module = load_module() + + with pytest.raises(ValueError, match="JSON serializable"): + module._validate_repository_dispatch_payload( + {"event_type": "bounded", "client_payload": {"value": object()}} + ) + + def test_dispatch_uses_central_events_and_acknowledges() -> None: """Both agents dispatch centrally with bounded review-only OpenCode options.""" @@ -327,12 +372,17 @@ def fake_run(command, **kwargs): monkeypatch.setattr(module.subprocess, "run", fake_run) client = module.GitHubClient("secret-token") - assert client.request(["repos/x/y"], input_payload={"a": 1}) == {"ok": True} + assert ( + client.request( + ["repos/x/y"], input_payload={"label": "한", "a": 1} + ) + == {"ok": True} + ) command, kwargs = calls[0] assert command == ["gh", "api", "repos/x/y", "--input", "-"] assert "secret-token" not in command assert kwargs["env"]["GH_TOKEN"] == "secret-token" - assert kwargs["input"] == '{"a": 1}' + assert kwargs["input"] == '{"label":"한","a":1}' monkeypatch.setattr( module.subprocess, "run", diff --git a/tests/test_agent_mention_workflow_contract.py b/tests/test_agent_mention_workflow_contract.py index c5fc4cae5..77546797d 100644 --- a/tests/test_agent_mention_workflow_contract.py +++ b/tests/test_agent_mention_workflow_contract.py @@ -17,6 +17,7 @@ def test_workflow_uses_local_event_and_central_sweep_with_job_scoped_writes() -> header, jobs = text.split("\njobs:\n", 1) assert "issue_comment:" in header assert 'cron: "*/5 * * * *"' in header + assert "concurrency:" not in header assert "workflow_dispatch:" not in header assert "permissions:\n contents: read" in header assert "contents: write" not in header diff --git a/tests/test_opencode_model_pool_runner.py b/tests/test_opencode_model_pool_runner.py index 08d17f000..5833220c3 100644 --- a/tests/test_opencode_model_pool_runner.py +++ b/tests/test_opencode_model_pool_runner.py @@ -157,14 +157,40 @@ def run_failed_model( fake_opencode.write_text( "#!/usr/bin/env bash\n" 'if [ "${1:-}" = run ]; then\n' - ' [ -z "${FAKE_OPENCODE_PROMPT_CAPTURE:-}" ] || printf \'%s\\n\' "$2" > "$FAKE_OPENCODE_PROMPT_CAPTURE"\n' + ' prompt="${2:-}"\n' + ' model=""\n' + ' while [ "$#" -gt 0 ]; do\n' + ' if [ "${1:-}" = "--model" ] && [ "$#" -ge 2 ]; then\n' + ' model="$2"\n' + ' shift 2\n' + ' else\n' + ' shift\n' + ' fi\n' + ' done\n' + ' [ -z "${FAKE_OPENCODE_MODEL_LOG:-}" ] || printf \'%s\\n\' "$model" >> "$FAKE_OPENCODE_MODEL_LOG"\n' + ' if [ "$model" = "${FAKE_OPENCODE_NEXT_MODEL:-}" ] && [ -f "${FAKE_OPENCODE_CHILD_PID_FILE:-}" ]; then\n' + ' child_pid="$(tr -d \'[:space:]\' < "$FAKE_OPENCODE_CHILD_PID_FILE")"\n' + ' if [ -n "$child_pid" ] && kill -0 "$child_pid" 2>/dev/null; then\n' + ' : > "${FAKE_OPENCODE_OVERLAP_FILE:?}"\n' + ' fi\n' + ' fi\n' + ' if [ "$model" = "${FAKE_OPENCODE_FATAL_MODEL:-}" ]; then\n' + ' (trap "" TERM; sleep "${FAKE_OPENCODE_CHILD_SLEEP_SECONDS:-120}") &\n' + ' child_pid=$!\n' + ' [ -z "${FAKE_OPENCODE_CHILD_PID_FILE:-}" ] || printf \'%s\' "$child_pid" > "$FAKE_OPENCODE_CHILD_PID_FILE"\n' + ' fi\n' + ' [ -z "${FAKE_OPENCODE_PROMPT_CAPTURE:-}" ] || printf \'%s\\n\' "$prompt" > "$FAKE_OPENCODE_PROMPT_CAPTURE"\n' ' [ -z "${FAKE_OPENCODE_JSON:-}" ] || printf \'%s\\n\' "$FAKE_OPENCODE_JSON"\n' ' [ -z "${FAKE_OPENCODE_STDERR:-}" ] || printf \'%s\\n\' "$FAKE_OPENCODE_STDERR" >&2\n' - ' sleep "${FAKE_OPENCODE_HANG_SECONDS:-0}"\n' + ' if [ -n "${FAKE_OPENCODE_FATAL_MODEL:-}" ] && [ "$model" != "$FAKE_OPENCODE_FATAL_MODEL" ]; then\n' + ' sleep "${FAKE_OPENCODE_NON_FATAL_HANG_SECONDS:-0}"\n' + ' else\n' + ' sleep "${FAKE_OPENCODE_HANG_SECONDS:-0}"\n' + ' fi\n' ' exit "${FAKE_OPENCODE_RUN_EXIT:-1}"\n' "fi\n" 'if [ "${1:-}" = export ]; then\n' - ' [ -z "${FAKE_OPENCODE_EXPORT:-}" ] || printf \'%s\\n\' "$FAKE_OPENCODE_EXPORT"\n' + ' if [ -n "${FAKE_OPENCODE_SUCCESS_EXPORT:-}" ]; then printf \'%s\\n\' "$FAKE_OPENCODE_SUCCESS_EXPORT"; else [ -z "${FAKE_OPENCODE_EXPORT:-}" ] || printf \'%s\\n\' "$FAKE_OPENCODE_EXPORT"; fi\n' ' exit "${FAKE_OPENCODE_EXPORT_EXIT:-0}"\n' "fi\n" "printf 'unexpected fake opencode command: %s\\n' \"$*\" >&2\n" @@ -547,6 +573,76 @@ def test_runner_never_cats_rejected_provider_artifacts() -> None: assert f'cat "${variable}"' not in runner +def test_process_group_launcher_tolerates_setsid_permission_error( + tmp_path: Path, +) -> None: + """A setsid failure still reaches the same provider exec path.""" + runner = RUNNER.read_text(encoding="utf-8") + function_start = runner.index("run_opencode_in_process_group() {") + function_end = runner.index("\nrun_one_model_attempt() {", function_start) + function_source = runner[function_start:function_end] + + prompt_file = tmp_path / "prompt.md" + prompt_file.write_text("review prompt", encoding="utf-8") + call_file = tmp_path / "exec-call.json" + sitecustomize = tmp_path / "sitecustomize.py" + sitecustomize.write_text( + """ +import json +import os +from pathlib import Path + + +def _raise_setsid() -> None: + raise PermissionError("test process-group permission") + + +def _record_exec(file: str, args: list[str], env: dict[str, str]) -> None: + Path(os.environ["SETSID_TEST_CALL_FILE"]).write_text( + json.dumps({"file": file, "args": args, "removed": "GITHUB_TOKEN" not in env}), + encoding="utf-8", + ) + raise SystemExit(0) + + +os.setsid = _raise_setsid +os.execvpe = _record_exec +""", + encoding="utf-8", + ) + harness = tmp_path / "run-launcher.sh" + harness.write_text( + "set -euo pipefail\n" + f"{function_source}\n" + f"run_opencode_in_process_group 7 {bash_path(prompt_file)!r} agent model title\n", + encoding="utf-8", + ) + + environment = os.environ.copy() + environment["PYTHONPATH"] = str(tmp_path) + environment["SETSID_TEST_CALL_FILE"] = str(call_file) + result = subprocess.run( + [bash_command(), bash_path(harness)], + capture_output=True, + text=True, + check=False, + env=environment, + timeout=10, + ) + + assert result.returncode == 0, result.stderr + assert "could not create an OpenCode process session" in result.stderr + recorded = json.loads(call_file.read_text(encoding="utf-8")) + assert recorded["file"] == "timeout" + assert recorded["args"][0:4] == [ + "timeout", + "--kill-after=30s", + "7s", + "opencode", + ] + assert recorded["removed"] is True + + @pytest.mark.parametrize( "json_line", [ @@ -583,6 +679,47 @@ def test_fatal_provider_error_kills_hung_opencode_run_early( assert elapsed < 25 +def test_fatal_cleanup_kills_term_ignoring_child_before_next_model( + tmp_path: Path, +) -> None: + """Captured process groups prevent a killed provider child from overlapping failover.""" + child_pid_file = tmp_path / "child.pid" + model_log = tmp_path / "models.log" + overlap_file = tmp_path / "overlap" + result = run_failed_model( + tmp_path, + json_line=( + '{"type":"error","error":{"name":"ProviderQuotaError","data":' + '{"message":"insufficient_quota: request rejected"}}}' + ), + model_candidates="openrouter/fatal openrouter/next", + extra_env={ + "OPENROUTER_API_KEY": "fake-openrouter-key", + "FAKE_OPENCODE_FATAL_MODEL": "openrouter/fatal", + "FAKE_OPENCODE_CHILD_PID_FILE": bash_path(child_pid_file), + "FAKE_OPENCODE_CHILD_SLEEP_SECONDS": "120", + "FAKE_OPENCODE_MODEL_LOG": bash_path(model_log), + "FAKE_OPENCODE_NEXT_MODEL": "openrouter/next", + "FAKE_OPENCODE_OVERLAP_FILE": bash_path(overlap_file), + "FAKE_OPENCODE_HANG_SECONDS": "120", + "FAKE_OPENCODE_NON_FATAL_HANG_SECONDS": "0", + "OPENCODE_RUN_TIMEOUT_SECONDS": "120", + "OPENCODE_TOTAL_RETRY_BUDGET_SECONDS": "240", + }, + ) + + assert result.returncode == 1 + assert model_log.read_text(encoding="utf-8").splitlines() == [ + "openrouter/fatal", + "openrouter/next", + ] + assert not overlap_file.exists() + child_pid = int(child_pid_file.read_text(encoding="utf-8")) + assert subprocess.run( + ["kill", "-0", str(child_pid)], check=False + ).returncode != 0 + + def test_model_text_quoting_error_signatures_does_not_kill_run(tmp_path: Path) -> None: """Model prose mentioning fatal signatures never kills a healthy streaming run.""" result = run_failed_model( diff --git a/tests/test_opencode_security_boundaries.py b/tests/test_opencode_security_boundaries.py index 1b22706fa..213ab4894 100644 --- a/tests/test_opencode_security_boundaries.py +++ b/tests/test_opencode_security_boundaries.py @@ -43,6 +43,13 @@ def test_sensitive_log_redaction_handles_json_credentials_and_jwts() -> None: assert set(json.loads(cleaned)["nested"].values()) == {redactor.REDACTED} +def test_sensitive_log_redaction_scrubs_non_sensitive_json_strings() -> None: + """Non-sensitive JSON strings still receive assignment and token redaction.""" + cleaned = redactor.redact_text(json.dumps({"message": "token=fixture-value"})) + + assert json.loads(cleaned)["message"] == f"token={redactor.REDACTED}" + + def test_sensitive_log_redaction_preserves_normal_diagnostics() -> None: """Ordinary failure reasons remain visible while credentials are removed.""" source = ( @@ -58,6 +65,39 @@ def test_sensitive_log_redaction_preserves_normal_diagnostics() -> None: assert cleaned.count(redactor.REDACTED) >= 2 +def test_sensitive_log_redaction_preserves_jwt_boundaries_and_operational_markers() -> None: + """JWT delimiters and allowlisted identifiers remain structurally readable.""" + operational = ( + "mail=user@example.test ip=192.0.2.10 phone=010-1234-5678 path=/tmp/secret" + ) + expected_operational = ( + "mail=[REDACTED_EMAIL] ip=[REDACTED_IP] " + "phone=[REDACTED_PHONE] path=[REDACTED_PATH]" + ) + assert redactor.redact_text("(header.payload.signature)") == ( + f"({redactor.REDACTED})" + ) + assert redactor._redact_operational_identifiers(operational) == ( + expected_operational + ) + cleaned = redactor.redact_text(operational) + assert cleaned == expected_operational + + json_cleaned = redactor._redact_unstructured( + json.dumps({"message": operational}), redact_assignments=False + ) + assert json.loads(json_cleaned)["message"] == expected_operational + + +def test_sensitive_log_redaction_handles_adjacent_ipv4_addresses() -> None: + """A delimiter must remain available for the next address match.""" + operational = "ips=192.0.2.10 198.51.100.2" + + assert redactor._redact_operational_identifiers(operational) == ( + "ips=[REDACTED_IP] [REDACTED_IP]" + ) + + def test_sensitive_log_redaction_handles_adversarial_quoted_values() -> None: """Quoted sensitive assignments are parsed linearly even with many escapes.""" source = "_jwt:\"" + "\\!" * 5000 diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index f2dd25813..a6add6c7c 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -1928,6 +1928,7 @@ def fake_run_with_env(args, *, stdin=None, env=None): "client_payload": { "target_repository": "owner/repo", "pr_number": 1, + "merge_state": "open", "pr_base_ref": "develop", "pr_base_sha": base_sha, "pr_head_sha": head_sha, @@ -1956,6 +1957,154 @@ def fake_run_with_env(args, *, stdin=None, env=None): } +def test_post_merge_strix_dispatch_binds_merged_target_tree(monkeypatch) -> None: + """Merged evidence uses the live merge commit without pre-merge base binding.""" + + calls = [] + monkeypatch.setattr(sched, "require_github_actions_control_actor", lambda _action: None) + monkeypatch.setattr(sched, "active_review_run_refs", lambda *_args, **_kwargs: ([], [])) + monkeypatch.setattr(sched, "force_cancel_workflow_run_refs", lambda _refs: None) + monkeypatch.setattr( + sched, + "run_github_dispatch", + lambda args, *, stdin=None: calls.append((args, stdin)), + ) + pr = make_pr( + state="CLOSED", + mergedAt="2026-08-15T04:05:06Z", + mergeCommit={"oid": "c" * 40}, + baseRefName="main", + headRefOid="a" * 40, + ) + + assert ( + sched.dispatch_post_merge_strix_evidence( + "owner/repo", + "Strix Security Scan", + pr, + dry_run=False, + ) + == "dispatched" + ) + payload = json.loads(calls[0][1]) + assert payload == { + "event_type": "strix-scan", + "client_payload": { + "target_repository": "owner/repo", + "pr_number": 1, + "merge_state": "merged", + "target_branch": "main", + "pr_head_sha": "a" * 40, + "merged_at": "2026-08-15T04:05:06Z", + "merged_commit_sha": "c" * 40, + }, + } + + +def test_post_merge_dispatch_rejects_unmerged_metadata() -> None: + """A closed-looking but unmerged PR cannot create post-merge evidence.""" + + with pytest.raises(ValueError, match="closed pull request"): + sched.validated_post_merge_dispatch_fields(make_pr()) + + with pytest.raises(ValueError, match="valid mergedAt"): + sched.validated_post_merge_dispatch_fields( + make_pr( + state="CLOSED", + mergedAt="not-a-timestamp", + mergeCommitOid="c" * 40, + headRefOid="a" * 40, + ) + ) + + assert sched.validated_post_merge_dispatch_fields( + make_pr( + state="CLOSED", + mergedAt="2026-08-15T04:05:06Z", + mergeCommitOid=None, + mergeCommit={"oid": "d" * 40}, + headRefOid="a" * 40, + ) + ) == ("main", "a" * 40, "2026-08-15T04:05:06Z", "d" * 40) + + +def test_post_merge_strix_dispatch_dry_run_and_active_run(monkeypatch, capsys) -> None: + """Post-merge dispatch remains bounded by dry-run and active-run guards.""" + + pr = make_pr( + state="CLOSED", + mergedAt="2026-08-15T04:05:06Z", + mergeCommitOid="c" * 40, + headRefOid="a" * 40, + ) + assert ( + sched.dispatch_post_merge_strix_evidence( + "owner/repo", "Strix Security Scan", pr, dry_run=True + ) + == "dry_run" + ) + + cancelled = [] + monkeypatch.setattr(sched, "require_github_actions_control_actor", lambda _action: None) + monkeypatch.setattr( + sched, + "active_review_run_refs", + lambda *_args, **_kwargs: ([ ("ContextualWisdomLab/.github", "123") ], [("ContextualWisdomLab/.github", "122")]), + ) + monkeypatch.setattr(sched, "force_cancel_workflow_run_refs", cancelled.extend) + monkeypatch.setattr( + sched, + "run_github_dispatch", + lambda *_args, **_kwargs: pytest.fail("active post-merge run must not dispatch again"), + ) + assert ( + sched.dispatch_post_merge_strix_evidence( + "owner/repo", "Strix Security Scan", pr, dry_run=False + ) + == "already_running" + ) + assert cancelled == [("ContextualWisdomLab/.github", "122")] + assert "Post-merge Strix dispatch skipped" in capsys.readouterr().out + + +def test_post_merge_dedup_ignores_pre_merge_same_head_run(monkeypatch) -> None: + """A queued open scan must not suppress the merged-tree evidence dispatch.""" + + head = "a" * 40 + monkeypatch.setattr(sched, "validate_github_repository", lambda value: value) + monkeypatch.setattr(sched, "repository_dispatch_target", lambda _value: "owner/repo") + monkeypatch.setattr( + sched, + "active_workflow_runs", + lambda *_args, **_kwargs: [ + { + "id": 1, + "name": "Strix Security Scan", + "event": "repository_dispatch", + "display_title": f"Strix Security Scan owner/repo#7@{head}:open", + }, + { + "id": 2, + "name": "Strix Security Scan", + "event": "repository_dispatch", + "display_title": f"Strix Security Scan owner/repo#7@{head}:merged", + }, + ], + ) + + current, stale = sched.active_review_run_refs( + "owner/repo", + "Strix Security Scan", + {"number": 7, "headRefOid": head}, + run_title="Strix Security Scan", + workflow_aliases=frozenset({"Strix Security Scan"}), + required_merge_state="merged", + ) + + assert current == [("owner/repo", "2")] + assert stale == [("owner/repo", "1")] + + def test_central_required_workflow_waits_without_cross_repo_dispatch_credential(monkeypatch): monkeypatch.setenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", "ContextualWisdomLab/.github") monkeypatch.setenv("SCHEDULER_REQUIRED_WORKFLOW_REF", "main") @@ -4509,6 +4658,59 @@ def fake_split_repo(repo, accepted_invalid=accepted_invalid): sched.main(["--repo", "owner/repo", "--base-branch", "main", "--project-flow", "github", "--pr-number", "-1"]) +def test_main_post_merge_dispatch_contract(monkeypatch, capsys): + """The post-merge CLI rejects ambiguity and publishes bounded outcomes.""" + + base_args = [ + "--repo", + "owner/repo", + "--base-branch", + "main", + "--project-flow", + "github", + "--post-merge", + ] + with pytest.raises(SystemExit, match="--post-merge requires --pr-number"): + sched.main(base_args) + + args = [*base_args, "--pr-number", "7"] + monkeypatch.setattr(sched, "fetch_pr", lambda _repo, _number: []) + with pytest.raises(SystemExit, match="exactly one pull request"): + sched.main(args) + + monkeypatch.setattr(sched, "fetch_pr", lambda _repo, _number: [make_pr(number=7)]) + monkeypatch.setattr( + sched, + "dispatch_post_merge_strix_evidence", + lambda *_args, **_kwargs: (_ for _ in ()).throw(RuntimeError("provider unavailable")), + ) + assert sched.main(args) == 1 + assert "failed closed: provider unavailable" in capsys.readouterr().err + + monkeypatch.setattr( + sched, + "dispatch_post_merge_strix_evidence", + lambda *_args, **_kwargs: "dry_run", + ) + assert sched.main(args) == 0 + dry_run_output = capsys.readouterr().out + assert "post-merge target-tree Strix evidence dry_run" in dry_run_output + assert json.loads(dry_run_output.strip().splitlines()[-1])["counts"] == { + "security_dispatch": 1 + } + + monkeypatch.setattr( + sched, + "dispatch_post_merge_strix_evidence", + lambda *_args, **_kwargs: "already_running", + ) + assert sched.main(args) == 0 + already_running_output = capsys.readouterr().out + assert json.loads(already_running_output.strip().splitlines()[-1])["counts"] == { + "wait": 1 + } + + def test_main_keeps_scanning_after_action_error(monkeypatch, capsys): assert sched.summarize_action_error(RuntimeError("")) == "scheduler action failed without stderr" diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 535fd513a..a038899ba 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1,3 +1,4 @@ +import hashlib import json import os import shlex @@ -85,6 +86,8 @@ def test_targeted_scheduler_dispatch_is_allowlisted_and_exact_pr_scoped() -> Non assert '--repo "$TARGET_REPOSITORY"' in inspect assert '--base-branch "$TARGET_DEFAULT_BRANCH"' in inspect assert 'args+=(--pr-number "$PULL_REQUEST_NUMBER")' in inspect + assert "POST_MERGE:" in workflow + assert "args+=(--post-merge)" in inspect assert ( "github.event_name == 'repository_dispatch' && " "github.event.client_payload.target_repository != '' && " @@ -1090,23 +1093,70 @@ def test_optional_strix_workflow_absence_is_logged_without_failing_lookup() -> N assert "skipping optional manual Strix run lookup" in workflow assert "Optional workflow %s is not installed" in failed_check_evidence assert 'if target_workflow_available "strix.yml"; then' in failed_check_evidence + assert "STRIX_BINDING_VALIDATION_CACHE" in failed_check_evidence + assert 'timeout -- "${timeout_seconds}s" gh api' in failed_check_evidence + assert 'timeout -- "${timeout_seconds}s" gh run download' in failed_check_evidence -def test_strix_provider_outage_without_findings_is_neutralized() -> None: +def test_strix_provider_outage_without_findings_fails_closed() -> None: + """Require the trusted gate to propagate every incomplete result.""" workflow = workflow_text("strix.yml") - assert "RateLimitError|Too many requests" in workflow - assert "exceeded your current quota" in workflow - assert "billing details" in workflow - assert "LLM warm-up failed" in workflow - assert "zero_vulnerabilities_signal" not in workflow - assert "(^|[^A-Za-z0-9_])severity[[:space:]]*:" in workflow + assert 'if [ "$strix_rc" -ne 0 ]; then' in workflow + assert 'exit "$strix_rc"' in workflow + assert "provider failures and missing reports remain fail-closed" in workflow + assert ( + 'grep -F -- "$STRIX_GATE_MARKER_PREFIX" "$strix_run_log"' + in workflow + ) + assert ( + "grep -Eiq 'failing closed|fail-closed|fail closed|incomplete evidence|incomplete-evidence|neutral[[:space:]]+skip'" + in workflow + ) + assert 'export STRIX_GATE_MARKER_PREFIX="CWL_STRIX_GATE_MARKER_${GITHUB_RUN_ID}:"' in workflow + assert "printed a fail-closed, incomplete-evidence, or neutral-skip marker but exited 0" in workflow + assert "Treating as a neutral skip" not in workflow + assert "backend_unavailable_signal" not in workflow + assert "reported_vulnerability_signal" not in workflow + assert "STRIX_FAIL_ON_PROVIDER_SIGNAL: \"1\"" in workflow assert "STRIX_FAIL_ON_MIN_SEVERITY: MEDIUM" in workflow - assert "before producing a vulnerability report" in workflow - assert "genuine findings still fail the check" in workflow + + +def test_strix_workflow_changes_require_post_merge_structured_evidence() -> None: + """Do not treat base-workflow false green as proof for workflow PRs.""" + strix_workflow = workflow_text("strix.yml") + failed_check_evidence = ( + REPO_ROOT / "scripts/ci/collect_failed_check_evidence.sh" + ).read_text(encoding="utf-8") + + assert "pull_request_target evaluates this workflow from the trusted base" in strix_workflow + assert "Materializing a PR-head workflow above is data-only self-test" in strix_workflow assert ( - '&& ! grep -Eiq "$reported_vulnerability_signal" "$strix_run_log"' in workflow + "Default-branch repository_dispatch Strix structured evidence binding passed" + in strix_workflow + ) + assert "TARGET_REPOSITORY:" in strix_workflow + assert "MERGE_STATE:" in strix_workflow + assert "SUPPLIED_TARGET_BRANCH:" in strix_workflow + assert "SUPPLIED_MERGED_COMMIT_SHA:" in strix_workflow + assert "post-merge repository_dispatch Strix metadata" in strix_workflow + assert 'trusted_workspace_sha="$live_merge_commit_sha"' in strix_workflow + assert "github.event.client_payload.merge_state != 'merged'" in strix_workflow + assert 'run_id="$GITHUB_RUN_ID"' in strix_workflow + assert "artifact_name:$artifact_name" in strix_workflow + assert "repository:$repository" in strix_workflow + for forbidden_report_path in ('*"/../"*', '*"/./"*', './*', '*//*'): + assert forbidden_report_path in failed_check_evidence + assert "/actions/runs/${run_id}/artifacts?per_page=100" in failed_check_evidence + assert "if ! artifact_count=\"$(jq -r" in failed_check_evidence + assert '.repository == $repository' in failed_check_evidence + assert '.artifact_name == "strix-reports"' in failed_check_evidence + redaction_step = workflow_step( + workflow_text("strix.yml"), + "Redact Strix evidence before artifact publication", ) + assert 'read_bytes().decode("utf-8")' in redaction_step + assert 'cp -- "$evidence_file" "$redacted_file"' in redaction_step def test_strix_cross_repo_dispatch_uses_target_token_for_pr_scoping() -> None: diff --git a/tests/test_strix_nvidia_nim_not_found_fallback.py b/tests/test_strix_nvidia_nim_not_found_fallback.py index a48f3092d..0d9af688e 100644 --- a/tests/test_strix_nvidia_nim_not_found_fallback.py +++ b/tests/test_strix_nvidia_nim_not_found_fallback.py @@ -43,15 +43,12 @@ def _function_block(source: str, function_name: str) -> str: return match.group(0) -def _classifies_as_nvidia_not_found(log_text: str) -> bool: - """Execute the production classifier against a bounded synthetic log.""" +def _classifies_with(function_name: str, log_text: str) -> bool: + """Execute one production Strix classifier against a bounded synthetic log.""" gate_source = STRIX_GATE.read_text(encoding="utf-8") - function_source = _function_block( - gate_source, - "is_nvidia_nim_not_found_error", - ) - with tempfile.TemporaryDirectory(prefix="strix-nvidia-404-") as temp_dir: + function_source = _function_block(gate_source, function_name) + with tempfile.TemporaryDirectory(prefix="strix-classifier-") as temp_dir: log_path = Path(temp_dir) / "strix.log" log_path.write_text(log_text, encoding="utf-8") script = "\n".join( @@ -59,7 +56,7 @@ def _classifies_as_nvidia_not_found(log_text: str) -> bool: "set -euo pipefail", 'STRIX_LOG="$1"', function_source, - "is_nvidia_nim_not_found_error", + function_name, ) ) completed = subprocess.run( @@ -73,50 +70,16 @@ def _classifies_as_nvidia_not_found(log_text: str) -> bool: return completed.returncode == 0 -def _workflow_signal_pattern(workflow: str, variable_name: str) -> str: - """Extract one single-quoted POSIX ERE assigned in the Strix workflow.""" +def _classifies_as_nvidia_not_found(log_text: str) -> bool: + """Execute the production NVIDIA 404 classifier.""" - match = re.search( - rf"(?m)^\s+{re.escape(variable_name)}='([^']+)'$", - workflow, - ) - if match is None: - raise AssertionError(f"missing workflow signal: {variable_name}") - return match.group(1) + return _classifies_with("is_nvidia_nim_not_found_error", log_text) -def _workflow_neutralizes(log_text: str) -> bool: - """Execute the outer workflow's backend-neutralization condition.""" +def _classifies_as_model_tool_contract(log_text: str) -> bool: + """Execute the production Strix tool-contract classifier.""" - workflow = STRIX_WORKFLOW.read_text(encoding="utf-8") - backend_pattern = _workflow_signal_pattern( - workflow, - "backend_unavailable_signal", - ) - vulnerability_pattern = _workflow_signal_pattern( - workflow, - "reported_vulnerability_signal", - ) - with tempfile.TemporaryDirectory(prefix="strix-workflow-404-") as temp_dir: - log_path = Path(temp_dir) / "strix.log" - log_path.write_text(log_text, encoding="utf-8") - backend = subprocess.run( - ["grep", "-Eiq", backend_pattern, str(log_path)], - check=False, - capture_output=True, - text=True, - ) - vulnerability = subprocess.run( - ["grep", "-Eiq", vulnerability_pattern, str(log_path)], - check=False, - capture_output=True, - text=True, - ) - if backend.returncode not in {0, 1}: - raise AssertionError(backend.stderr) - if vulnerability.returncode not in {0, 1}: - raise AssertionError(vulnerability.stderr) - return backend.returncode == 0 and vulnerability.returncode == 1 + return _classifies_with("is_strix_model_tool_contract_error", log_text) class StrixNvidiaNotFoundFallbackTests(unittest.TestCase): @@ -171,6 +134,41 @@ def test_not_found_skips_same_model_and_enters_cross_model_fallback(self) -> Non self.assertIn("is_nvidia_nim_not_found_error", retryable) self.assertNotIn("is_nvidia_nim_not_found_error", same_model_retry) + def test_unsupported_tool_contract_enters_cross_model_fallback(self) -> None: + """Treat the Strix agent/tool mismatch as a model failure, not a finding.""" + + log = ( + "Traceback (most recent call last):\n" + ' File "/site-packages/strix/core/execution.py", line 355, in _run_cycle\n' + ' File "/site-packages/agents/run_internal/turn_resolution.py", ' + 'line 1828, in process_model_response\n' + "agents.exceptions.ModelBehaviorError: Tool execute not found in agent strix\n" + ) + self.assertTrue(_classifies_as_model_tool_contract(log)) + + gate_source = STRIX_GATE.read_text(encoding="utf-8") + infrastructure = _function_block( + gate_source, + "has_detected_infrastructure_error", + ) + retryable = _function_block(gate_source, "is_model_retryable_error") + same_model_retry = _function_block( + gate_source, + "is_transient_same_model_retry_error", + ) + self.assertIn("is_strix_model_tool_contract_error", infrastructure) + self.assertIn("is_strix_model_tool_contract_error", retryable) + self.assertNotIn("is_strix_model_tool_contract_error", same_model_retry) + + def test_target_text_cannot_spoof_tool_contract_fallback(self) -> None: + """Require the Strix traceback marker beside the exact exception.""" + + log = ( + "source literal: agents.exceptions.ModelBehaviorError: Tool execute " + "not found in agent strix\n" + ) + self.assertFalse(_classifies_as_model_tool_contract(log)) + def test_workflow_uses_available_free_first_nvidia_plan(self) -> None: """Prefer a documented hosted NIM and another NIM before GitHub.""" @@ -199,62 +197,28 @@ def test_workflow_uses_available_free_first_nvidia_plan(self) -> None: )[0] self.assertNotIn(RETIRED_PRIMARY_MODEL, default_gate) - def test_outer_workflow_requires_litellm_context_for_nvidia_404(self) -> None: - """Reject provider-like target text in the outer neutralization gate.""" + def test_workflow_propagates_provider_404_gate_failures(self) -> None: + """Do not let the outer workflow neutralize provider 404 evidence.""" - self.assertFalse( - _workflow_neutralizes( - "source literal: Nvidia_nimException Error code: 404\n" - ) - ) - self.assertTrue( - _workflow_neutralizes( - "litellm.exceptions.NotFoundError: Nvidia_nimException - " - "Error code: 404\nVulnerabilities 0\n" - ) - ) - - def test_outer_workflow_rejects_cross_line_signal_assembly(self) -> None: - """Require exception, provider, and 404 evidence on one physical line.""" - - self.assertFalse( - _workflow_neutralizes( - "litellm.exceptions.NotFoundError: provider unavailable\n" - "Nvidia_nimException Error code: 404\n" - ) - ) - - def test_outer_workflow_rejects_nvidia_404_without_litellm_context(self) -> None: - """Require LiteLLM NotFoundError context, not just NVIDIA + 404.""" - - self.assertFalse( - _workflow_neutralizes( - "Nvidia_nimException Error code: 404\nVulnerabilities 0\n" - ) + workflow = STRIX_WORKFLOW.read_text(encoding="utf-8") + self.assertIn('if [ "$strix_rc" -ne 0 ]; then', workflow) + self.assertIn('exit "$strix_rc"', workflow) + self.assertIn( + "provider failures and missing reports remain fail-closed", + workflow, ) - - def test_outer_workflow_never_neutralizes_reported_vulnerabilities(self) -> None: - """Keep a real vulnerability signal blocking despite provider failure.""" - - self.assertFalse( - _workflow_neutralizes( - "litellm.exceptions.NotFoundError: Nvidia_nimException - " - "Error code: 404\nVulnerabilities 1\n" - ) + self.assertIn( + 'grep -F -- "$STRIX_GATE_MARKER_PREFIX" "$strix_run_log"', + workflow, ) - - def test_workflow_neutralizes_only_nvidia_404_without_findings(self) -> None: - """Retain the static fail-closed vulnerability evidence contract.""" - - workflow = STRIX_WORKFLOW.read_text(encoding="utf-8") - self.assertIn("Nvidia_nimException", workflow) - self.assertIn("Error code:[[:space:]]*404", workflow) - self.assertIn("reported_vulnerability_signal", workflow) - self.assertIn("Vulnerabilities[[:space:]]+[1-9]", workflow) self.assertIn( - '! grep -Eiq "$reported_vulnerability_signal"', + "grep -Eiq 'failing closed|fail-closed|fail closed|incomplete evidence|incomplete-evidence|neutral[[:space:]]+skip'", workflow, ) + self.assertIn("neutral[[:space:]]+skip", workflow) + self.assertNotIn("backend_unavailable_signal", workflow) + self.assertNotIn("reported_vulnerability_signal", workflow) + self.assertNotIn("Treating as a neutral skip", workflow) if __name__ == "__main__":