diff --git a/.github/workflows/codeql-pr.yml b/.github/workflows/codeql-pr.yml index 240e7139d..7281fb618 100644 --- a/.github/workflows/codeql-pr.yml +++ b/.github/workflows/codeql-pr.yml @@ -194,7 +194,90 @@ jobs: uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: persist-credentials: false - ref: ${{ format('refs/pull/{0}/merge', github.event.pull_request.number) }} + # GitHub can omit the synthetic merge ref for stacked/non-default-base PRs; + # materialize the exact merge-result SHA locally while retaining the + # documented pull-request ref shape for CodeQL result publication. + ref: ${{ github.event.pull_request.merge_commit_sha }} + # merge-tree requires the real base/head merge base; an explicit-SHA + # depth-1 checkout leaves every fetched commit as a shallow root. + fetch-depth: 0 + + - name: Verify merge preview identity + id: verify-merge-preview + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + MERGE_SHA: ${{ github.event.pull_request.merge_commit_sha }} + PR_NUMBER: ${{ github.event.pull_request.number }} + GITHUB_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + export GIT_CONFIG_COUNT=1 + export GIT_CONFIG_KEY_0=http.extraheader + export GIT_CONFIG_VALUE_0="AUTHORIZATION: bearer $GITHUB_TOKEN" + git fetch --no-tags origin "$BASE_SHA" "$MERGE_SHA" + head_fetch_succeeded=0 + if git cat-file -e "$HEAD_SHA^{commit}"; then + head_fetch_succeeded=1 + elif git fetch --no-tags origin "$HEAD_SHA" && + git cat-file -e "$HEAD_SHA^{commit}"; then + head_fetch_succeeded=1 + else + echo "Direct CodeQL head-SHA fetch was unavailable; resolving the exact head through the target pull request ref." + fi + if [ "$head_fetch_succeeded" -ne 1 ]; then + for pr_head_fetch_attempt in 1 2 3 4 5 6; do + if git fetch --no-tags --prune origin \ + "+refs/pull/${PR_NUMBER}/head:refs/remotes/origin/pr-${PR_NUMBER}-head"; then + fetched_head_sha="$(git rev-parse "refs/remotes/origin/pr-${PR_NUMBER}-head")" + if [ "$fetched_head_sha" = "$HEAD_SHA" ]; then + head_fetch_succeeded=1 + break + fi + if [ "$pr_head_fetch_attempt" -lt 6 ]; then + echo "Fetched PR head $fetched_head_sha, expected $HEAD_SHA; retrying after propagation delay." + sleep 10 + fi + elif [ "$pr_head_fetch_attempt" -lt 6 ]; then + echo "PR head ref fetch failed on attempt $pr_head_fetch_attempt; retrying after propagation delay." + sleep 10 + fi + done + fi + if [ "$head_fetch_succeeded" -ne 1 ] || + ! git cat-file -e "$HEAD_SHA^{commit}"; then + echo "::error::CodeQL merge preview could not resolve exact head SHA $HEAD_SHA from origin or refs/pull/$PR_NUMBER/head." + exit 1 + fi + unset GIT_CONFIG_COUNT GIT_CONFIG_KEY_0 GIT_CONFIG_VALUE_0 + provided_tree="$(git rev-parse "$MERGE_SHA^{tree}")" + read -r actual_merge parent_one parent_two extra <> "$GITHUB_OUTPUT" + echo "Verified merge preview: base=$BASE_SHA head=$HEAD_SHA provided_merge=$MERGE_SHA analyzed_merge=$local_merge_sha tree=$expected_tree" - name: Initialize CodeQL uses: github/codeql-action/init@99df26d4f13ea111d4ec1a7dddef6063f76b97e9 # v4.37.0 @@ -209,7 +292,7 @@ jobs: upload: false output: codeql-results-merge ref: ${{ format('refs/pull/{0}/merge', github.event.pull_request.number) }} - sha: ${{ github.event.pull_request.merge_commit_sha }} + sha: ${{ steps.verify-merge-preview.outputs.merge_sha }} - name: Enforce CodeQL Medium+ SARIF gate shell: python3 {0} diff --git a/.github/workflows/opencode-review-dispatch.yml b/.github/workflows/opencode-review-dispatch.yml index 0df7a17cc..978845efe 100644 --- a/.github/workflows/opencode-review-dispatch.yml +++ b/.github/workflows/opencode-review-dispatch.yml @@ -185,7 +185,7 @@ jobs: ! [[ "$live_is_private" =~ ^(true|false)$ ]] || [ -z "$live_base_ref" ] || [ -z "$live_head_ref" ]; then - printf '::error::PR metadata validation rejected closed, missing, or malformed live metadata. target=%s#%s state=%s base_repo=%s head_repo=%s base=%s head=%s\n' "$TARGET_REPOSITORY" "$PR_NUMBER" "${live_state:-}" "${live_base_repository:-}" "${live_head_repository:-}" "${live_base_sha:-}" "${live_head_sha:-}" + printf '::error::PR metadata validation rejected closed, missing, malformed, or base-repository-mismatched live metadata. target=%s#%s state=%s base_repo=%s head_repo=%s base=%s head=%s\n' "$TARGET_REPOSITORY" "$PR_NUMBER" "${live_state:-}" "${live_base_repository:-}" "${live_head_repository:-}" "${live_base_sha:-}" "${live_head_sha:-}" exit 1 fi @@ -333,8 +333,44 @@ jobs: git -C "$fetch_dir" remote add origin "${GITHUB_SERVER_URL}/${TARGET_REPOSITORY}.git" if ! git -C "$fetch_dir" \ -c http.extraheader="AUTHORIZATION: basic ${auth_header}" \ - fetch --no-tags --prune --no-recurse-submodules origin "$PR_BASE_SHA" "$PR_HEAD_SHA"; then - echo "::error::Coverage fetch could not authenticate to ${TARGET_REPOSITORY} or read base/head SHAs ${PR_BASE_SHA}/${PR_HEAD_SHA}; check token permissions, target repository access, and SHA visibility." + fetch --no-tags --prune --no-recurse-submodules origin "$PR_BASE_SHA"; then + echo "::error::Coverage fetch could not authenticate to ${TARGET_REPOSITORY} or read base SHA ${PR_BASE_SHA}; check token permissions, target repository access, and SHA visibility." + exit 1 + fi + head_fetch_succeeded=0 + if git -C "$fetch_dir" \ + -c http.extraheader="AUTHORIZATION: basic ${auth_header}" \ + fetch --no-tags --prune --no-recurse-submodules origin "$PR_HEAD_SHA"; then + head_fetch_succeeded=1 + else + echo "Direct head-SHA coverage fetch was unavailable; resolving the exact head through the target pull request ref." + fi + if [ "$head_fetch_succeeded" -ne 1 ]; then + for pr_head_fetch_attempt in 1 2 3 4 5 6; do + if git -C "$fetch_dir" \ + -c http.extraheader="AUTHORIZATION: basic ${auth_header}" \ + fetch --no-tags --prune --no-recurse-submodules origin \ + "+refs/pull/${PR_NUMBER}/head:refs/remotes/origin/pr-${PR_NUMBER}-head"; then + fetched_head_sha="$(git -C "$fetch_dir" rev-parse "refs/remotes/origin/pr-${PR_NUMBER}-head")" + if [ "$fetched_head_sha" = "$PR_HEAD_SHA" ]; then + head_fetch_succeeded=1 + break + fi + if [ "$pr_head_fetch_attempt" -lt 6 ]; then + echo "Fetched PR head $fetched_head_sha, expected $PR_HEAD_SHA; retrying after propagation delay." + sleep 10 + else + echo "Fetched PR head $fetched_head_sha, expected $PR_HEAD_SHA; no retries remain." + fi + elif [ "$pr_head_fetch_attempt" -lt 6 ]; then + echo "PR head ref fetch failed on attempt $pr_head_fetch_attempt; retrying after propagation delay." + sleep 10 + fi + done + fi + if [ "$head_fetch_succeeded" -ne 1 ] || + ! git -C "$fetch_dir" cat-file -e "${PR_HEAD_SHA}^{commit}"; then + echo "::error::Coverage fetch could not resolve exact head SHA ${PR_HEAD_SHA} from ${TARGET_REPOSITORY} or refs/pull/${PR_NUMBER}/head; check token permissions, target repository access, PR state, and SHA propagation." exit 1 fi git -C "$fetch_dir" checkout --detach "$PR_BASE_SHA" @@ -609,6 +645,7 @@ jobs: coverage_build_dir="${RUNNER_TEMP}/opencode-coverage-tool-build" trusted_ci_requirements="${GITHUB_WORKSPACE}/requirements-opencode-review-ci-hashes.txt" trusted_base_python_installer="${GITHUB_WORKSPACE}/scripts/ci/install_base_python_locks.py" + trusted_vcs_license_validator="${GITHUB_WORKSPACE}/scripts/ci/validate_vcs_dependency_license.py" if [ ! -f "$trusted_ci_requirements" ] || [ -L "$trusted_ci_requirements" ]; then echo "::error::Trusted coverage requirements must be a regular non-symlink file." exit 1 @@ -617,6 +654,10 @@ jobs: echo "::error::Trusted base Python lock installer must be a regular non-symlink file." exit 1 fi + if [ ! -f "$trusted_vcs_license_validator" ] || [ -L "$trusted_vcs_license_validator" ]; then + echo "::error::Trusted VCS dependency license validator must be a regular non-symlink file." + exit 1 + fi sudo rm -rf "$coverage_build_dir" mkdir -p "$coverage_build_dir" chmod 0700 "$coverage_build_dir" @@ -624,6 +665,8 @@ jobs: "$coverage_build_dir/requirements-opencode-review-ci-hashes.txt" install -m 0755 "$trusted_base_python_installer" \ "$coverage_build_dir/install-base-python-locks.py" + install -m 0755 "$trusted_vcs_license_validator" \ + "$coverage_build_dir/validate-vcs-dependency-license.py" python3 -I "$GITHUB_WORKSPACE/scripts/ci/materialize_base_python_requirements.py" \ --repo-root "$COVERAGE_SOURCE_WORKDIR" \ --base-sha "$PR_BASE_SHA" \ @@ -720,6 +763,7 @@ jobs: -r /tmp/requirements-opencode-review-ci-hashes.txt \ && rm -f /tmp/requirements-opencode-review-ci-hashes.txt COPY base-python-requirements /tmp/base-python-requirements + COPY validate-vcs-dependency-license.py /usr/local/libexec/validate-vcs-dependency-license.py RUN set -eu; \ mkdir -p /opt/base-vcs-dependencies; \ site_packages="$(python3 -c 'import site; print(site.getsitepackages()[0])')"; \ @@ -730,6 +774,10 @@ jobs: jq -r '.[] | [.import_name, .repository, .commit] | @tsv' \ /tmp/base-python-requirements/vcs-manifest.json >"$dependency_list"; \ while IFS="$(printf '\t')" read -r import_name repository commit; do \ + license_spdx="$(python3 -I /usr/local/libexec/validate-vcs-dependency-license.py \ + --repository "$repository" --commit "$commit")"; \ + printf 'Validated permitted VCS dependency license: repository=%s commit=%s SPDX=%s\n' \ + "$repository" "$commit" "$license_spdx"; \ destination="$(printf '/opt/base-vcs-dependencies/dependency-%03d' "$dependency_index")"; \ git init --quiet "$destination"; \ git -C "$destination" remote add origin \ @@ -887,6 +935,11 @@ jobs: : >"$GITHUB_OUTPUT" chmod 0600 "$GITHUB_OUTPUT" unset ACTIONS_ID_TOKEN_REQUEST_TOKEN ACTIONS_ID_TOKEN_REQUEST_URL ACTIONS_RUNTIME_TOKEN GH_TOKEN GITHUB_TOKEN + # Corepack may resolve a package-manager release from package.json. + # The validated base/head identity check below makes that selection + # immutable, and this keeps execution offline even if Corepack would + # otherwise attempt a download. + export COREPACK_ENABLE_NETWORK=0 umask 077 cd "$COVERAGE_SOURCE_WORKDIR" @@ -1430,6 +1483,49 @@ jobs: writable_npm_cache_dir="$destination" } + trusted_pnpm_package_manager_matches_base() { + local relative_dir + local relative_manifest + local base_spec + local head_spec + local worktree_spec + + case "$PWD" in + "$COVERAGE_SOURCE_WORKDIR") + relative_dir="" + ;; + "$COVERAGE_SOURCE_WORKDIR"/*) + relative_dir="${PWD#"$COVERAGE_SOURCE_WORKDIR"/}" + ;; + *) + echo "::error::pnpm project directory escaped the validated coverage worktree." >&2 + return 1 + ;; + esac + relative_manifest="${relative_dir:+${relative_dir}/}package.json" + if [ ! -f package.json ] || [ -L package.json ]; then + echo "::error::Current pnpm package manifest must be a regular non-symlink file." >&2 + return 1 + fi + + base_spec="$(trusted_git show "${PR_BASE_SHA}:${relative_manifest}" | jq -er '.packageManager // empty')" || { + echo "::error::Validated base does not declare an exact pnpm packageManager in ${relative_manifest}." >&2 + return 1 + } + head_spec="$(trusted_git show "${PR_HEAD_SHA}:${relative_manifest}" | jq -er '.packageManager // empty')" || { + echo "::error::Validated head does not declare an exact pnpm packageManager in ${relative_manifest}." >&2 + return 1 + } + worktree_spec="$(jq -er '.packageManager // empty' package.json)" || { + echo "::error::Current pnpm package manifest does not declare packageManager." >&2 + return 1 + } + if [ "$base_spec" != "$head_spec" ] || [ "$head_spec" != "$worktree_spec" ]; then + echo "::error::Current pnpm packageManager specification differs from the validated base; refusing Corepack version resolution." >&2 + return 1 + fi + } + trusted_pnpm_lock_matches_base() { local relative_dir local relative_lock @@ -1557,6 +1653,7 @@ jobs: fi ;; pnpm) + trusted_pnpm_package_manager_matches_base trusted_pnpm_lock_matches_base prepare_writable_pnpm_store if pnpm_supports_trust_lockfile; then @@ -2322,7 +2419,7 @@ jobs: ! [[ "$EXPECTED_IS_PRIVATE" =~ ^(true|false)$ ]] || ! [[ "$live_is_private" =~ ^(true|false)$ ]] || [ "$live_is_private" != "$EXPECTED_IS_PRIVATE" ]; then - printf '::error::OpenCode privileged review metadata changed before OIDC, review-token, CodeGraph, or model execution. target=%s#%s state=%s base_repo=%s base=%s/%s expected_base=%s/%s head_repo=%s head=%s/%s expected_head=%s/%s private=%s expected_private=%s\n' \ + printf '::error::OpenCode privileged review metadata changed before OIDC, review-token, CodeGraph, or model execution; exact base/head identity no longer matches the validated request. target=%s#%s state=%s base_repo=%s base=%s/%s expected_base=%s/%s head_repo=%s head=%s/%s expected_head=%s/%s private=%s expected_private=%s\n' \ "$GH_REPOSITORY" "$PR_NUMBER" "${live_state:-}" "${base_repository:-}" "${live_base_ref:-}" "${live_base_sha:-}" "$EXPECTED_BASE_REF" "$EXPECTED_BASE_SHA" "${head_repository:-}" "${live_head_ref:-}" "${live_head_sha:-}" "$EXPECTED_HEAD_REF" "$EXPECTED_HEAD_SHA" "${live_is_private:-}" "${EXPECTED_IS_PRIVATE:-}" exit 1 fi diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 7f1ad6d00..bd074f7ae 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -20,6 +20,7 @@ concurrency: permissions: contents: read + pull-requests: read jobs: required-workflow-bootstrap: @@ -53,7 +54,56 @@ jobs: name: opencode-review needs: [coverage-evidence] runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read steps: - - run: >- - echo "Review approval remains a separate current-head PR review - requirement produced by the authenticated dispatch workflow." + - name: Fail closed without a current-head OpenCode verdict + env: + GH_TOKEN: ${{ github.token }} + TARGET_REPOSITORY: ${{ github.event.pull_request.base.repo.full_name || github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + set -euo pipefail + if [ "${{ github.event.action }}" = "closed" ]; then + echo "PR closed; a current-head OpenCode verdict is not required." + exit 0 + fi + if [ -z "${PR_NUMBER:-}" ] || [ -z "${HEAD_SHA:-}" ]; then + echo "::error::Missing PR number or head SHA; cannot verify a current-head OpenCode verdict." + exit 1 + fi + reviews="$(gh api --paginate "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}/reviews")" + verdict="$(printf '%s\n' "$reviews" | jq -r -s --arg sha "$HEAD_SHA" ' + (add // []) + | [ + .[] + | select( + (.user.login // "" | ascii_downcase) as $user + | $user == "opencode-agent" or $user == "opencode-agent[bot]" + ) + | select((.commit_id // "" | ascii_downcase) == ($sha | ascii_downcase)) + ] + | (last // {}) as $review + | ($review.body // "" | ascii_downcase) as $body + | if $review.state == "CHANGES_REQUESTED" then + "CHANGES_REQUESTED" + elif $review.state == "APPROVED" + and ($body | contains("deterministic current-head evidence") | not) + and ($body | contains("deterministic fallback approval") | not) + and ($body | contains("model-unavailable evidence fallback") | not) + and ($body | contains("did not emit a usable current-head control block") | not) + and ($body | contains("scope: `unsupported`") | not) + and ($body | contains("model-pool outcome: `unknown`") | not) + then + "APPROVED" + else + empty + end + ')" + if [ -z "$verdict" ]; then + echo "::error::No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head. This required check is not a review and must not succeed until the authenticated dispatch posts a current-head verdict." + exit 1 + fi + echo "Current-head OpenCode verdict: ${verdict}." diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index a9bb54f8a..e5d3a5052 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -92,7 +92,7 @@ concurrency: github.event_name == 'pull_request_target' && format('pr-{0}', github.event.pull_request.number) || github.event_name == 'pull_request_review' && format('pr-{0}', github.event.pull_request.number) || github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].number && format('pr-{0}', github.event.workflow_run.pull_requests[0].number) || - github.event_name == 'workflow_run' && !github.event.workflow_run.pull_requests[0].number && format('workflow-run-no-pr-{0}', github.repository) || + github.event_name == 'workflow_run' && format('workflow-run-{0}', github.ref) || github.event_name == 'workflow_call' && inputs.pr_number != '' && format('pr-{0}', inputs.pr_number) || github.event_name == 'workflow_call' && inputs.base_branch != '' && format('call-{0}', inputs.base_branch) || github.event_name == 'schedule' && format('schedule-{0}', github.event.schedule) || @@ -246,6 +246,7 @@ jobs: { printf 'repository=%s\n' "$GITHUB_REPOSITORY" printf 'base_branch=%s\n' "$DEFAULT_BRANCH" + printf 'default_branch=%s\n' "$DEFAULT_BRANCH" } >>"$GITHUB_OUTPUT" exit 0 fi @@ -277,21 +278,26 @@ jobs: fi pull_json="$(gh api "repos/${TARGET_REPOSITORY_INPUT}/pulls/${TARGET_PR_NUMBER}")" + repository_json="$(gh api "repos/${TARGET_REPOSITORY_INPUT}")" live_number="$(jq -r '.number // 0' <<<"$pull_json")" live_state="$(jq -r '.state // empty' <<<"$pull_json")" live_base_repository="$(jq -r '.base.repo.full_name // empty' <<<"$pull_json")" live_head_repository="$(jq -r '.head.repo.full_name // empty' <<<"$pull_json")" live_base_branch="$(jq -r '.base.ref // empty' <<<"$pull_json")" + live_default_branch="$(jq -r '.default_branch // empty' <<<"$repository_json")" live_head_sha="$(jq -r '.head.sha // empty' <<<"$pull_json")" if [ "$live_number" != "$TARGET_PR_NUMBER" ] || [ "$live_state" != "open" ] || [ "$live_base_repository" != "$TARGET_REPOSITORY_INPUT" ] || ! [[ "$live_head_repository" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]] || [ -z "$live_base_branch" ] || + [ -z "$live_default_branch" ] || ! [[ "$live_head_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then - printf '::error::Targeted scheduler dispatch rejected closed or malformed live PR metadata. target=%s pr=%s state=%s base_repository=%s head_repository=%s base_branch=%s head_sha=%s\n' "$TARGET_REPOSITORY_INPUT" "$TARGET_PR_NUMBER" "${live_state:-}" "${live_base_repository:-}" "${live_head_repository:-}" "${live_base_branch:-}" "${live_head_sha:-}" + printf '::error::Targeted scheduler dispatch rejected closed, malformed, or base-repository-mismatched live PR metadata. target=%s pr=%s state=%s base_repository=%s head_repository=%s base_branch=%s default_branch=%s head_sha=%s\n' "$TARGET_REPOSITORY_INPUT" "$TARGET_PR_NUMBER" "${live_state:-}" "${live_base_repository:-}" "${live_head_repository:-}" "${live_base_branch:-}" "${live_default_branch:-}" "${live_head_sha:-}" exit 1 fi + printf 'Validated exact-PR targeted scheduler dispatch for %s#%s (head=%s).\n' \ + "$TARGET_REPOSITORY_INPUT" "$TARGET_PR_NUMBER" "$live_head_repository" if [ -n "$TARGET_BASE_BRANCH_INPUT" ] && [ "$TARGET_BASE_BRANCH_INPUT" != "$live_base_branch" ]; then printf '::error::Targeted scheduler dispatch base branch does not match the live PR. supplied=%s live=%s\n' "$TARGET_BASE_BRANCH_INPUT" "$live_base_branch" @@ -301,9 +307,10 @@ jobs: { printf 'repository=%s\n' "$TARGET_REPOSITORY_INPUT" printf 'base_branch=%s\n' "$live_base_branch" + printf 'default_branch=%s\n' "$live_default_branch" printf 'head_sha=%s\n' "$live_head_sha" } >>"$GITHUB_OUTPUT" - printf 'Validated exact targeted scheduler dispatch for %s#%s at %s on base %s.\n' "$TARGET_REPOSITORY_INPUT" "$TARGET_PR_NUMBER" "$live_head_sha" "$live_base_branch" + printf 'Validated exact targeted scheduler dispatch for %s#%s at %s on base %s; repository default is %s.\n' "$TARGET_REPOSITORY_INPUT" "$TARGET_PR_NUMBER" "$live_head_sha" "$live_base_branch" "$live_default_branch" - name: Resolve trusted scheduler source ref id: trusted_source @@ -1157,6 +1164,47 @@ jobs: } ]' <<<"$active_runs_json" )" + + # Metadata-free workflow_run events use a dedicated concurrency + # group, while this sweep keeps only the newest scan for the + # current default HEAD and cancels older same-head scans left by + # a queue backlog or an earlier scheduler implementation. + duplicate_scheduler_runs_json="[]" + if [ -n "$current_default_sha" ]; then + duplicate_scheduler_runs_json="$( + jq \ + --arg default_branch "$default_branch" \ + --arg current_default_sha "$current_default_sha" \ + '[ + .[] + | select( + .event == "workflow_run" and + .name == "Required PR Review Merge Scheduler" and + .head_branch == $default_branch and + .head_sha == $current_default_sha and + ((.pull_requests // []) | length) == 0 + ) + | { + id, + name, + status, + event, + head_branch, + run_head: .head_sha, + current_head: $current_default_sha, + created_at + } + ] + | sort_by(.created_at, .id) + | .[0:-1]' <<<"$active_runs_json" + )" + superseded_runs_json="$( + jq -n \ + --argjson superseded "$superseded_runs_json" \ + --argjson duplicates "$duplicate_scheduler_runs_json" \ + '$superseded + $duplicates | unique_by(.id)' + )" + fi fi superseded_count="$(jq 'length' <<<"$superseded_runs_json")" if [ "$superseded_count" -gt 0 ]; then diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 6310abcfe..2cbef4eb1 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -116,6 +116,14 @@ sequenceDiagram MS->>PR: merge only on current-head approval + green checks ``` +The central scheduler also performs fail-closed queue hygiene. A +`workflow_run` event without PR metadata uses a workflow-run-specific +default-branch fallback, separate from push runs; the workflow cancels older +runs in that group, and the organization sweep retains only the newest +metadata-free scheduler scan for the current default-branch SHA. +PR-associated and unrelated workflow runs are not included in this +deduplication. + ## Trust boundaries - Required review workflows execute **base-branch** scripts. A PR that edits diff --git a/CHANGELOG.md b/CHANGELOG.md index cef0acda6..672cc1e05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,14 @@ Semantic Versioning where the repository publishes a release. ### Changed +- Noema now validates the current-head primary OpenCode approval before accepting an existing Noema verdict, preventing a secondary review from making the required gate look successful on its own. +- Draft pull requests now receive same-head Strix and OpenCode review dispatches while remaining excluded from branch updates, auto-merge changes, direct merge, and review-state cleanup. +- The required `opencode-review` check now fails closed unless `opencode-agent` already posted `APPROVED` or `CHANGES_REQUESTED` on the current head, so a stub success can no longer look like a review (ContextualWisdomLab/contextual-orchestrator#176). +- The merge scheduler now spends its review-dispatch budget on pull requests with no OpenCode verdict on any commit before leftover increments that already have a previous-head APPROVED or CHANGES_REQUESTED, so one-dispatch-per-run no longer starves an empty Reviews tab. +- The scheduler treats GitHub's full `run-name` (`OpenCode Review Dispatch owner/repo#N@sha`) as an in-progress same-head dispatch, so a later sweep cannot `cancel-in-progress` a review that already passed coverage. +- Noema no longer exits 0 when the current head has no primary OpenCode approval, including on draft pull requests; that skip was the green `noema-review` check with an empty Reviews tab. +- `load_codegraph_context` now confines `NOEMA_CODEGRAPH_CONTEXT_PATH` to `GITHUB_WORKSPACE` (or cwd) with `..` rejection and realpath checks, so a Strix path-traversal report on that helper cannot read files outside the review workspace. +- Deduplicate central `workflow_run` scheduler scans: metadata-free workflow-run events now cancel an older run in a workflow-run-specific branch fallback, and the organization queue sweep retains only the newest metadata-free scheduler scan for the current default-branch HEAD while preserving push, PR-associated, and unrelated workflow runs. - Emit completed repository pull-list requests as they finish in the five-minute agent-mention sweep, while retaining the four-worker ceiling, rotation, and exact-name dispatch ledger, so one slow repository cannot hide ready sibling @@ -81,6 +89,7 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Made the CodeQL pull-request gate bind analysis to the exact current base/head merge tree: when GitHub's `pull_request.merge_commit_sha` is stale or structurally different, the workflow now materializes a deterministic local two-parent merge preview and publishes the analyzed SHA instead of failing against an older revision. - Resolve Strix visibility from the trusted GitHub event for ordinary push, schedule, and pull-request runs, reserving API retries for cross-repository dispatches whose workflow token may not see the target repository. @@ -161,6 +170,7 @@ Semantic Versioning where the repository publishes a release. ### Security +- Preserve fork heads as untrusted review-only inputs for allowlisted base repositories: privileged review revalidates exact base/head identity, while branch mutation and automatic or final external-head merge remain maintainer-controlled. - Keep the Quarantine Sandbox Runtime caller read-only and model-secret-free, grant only job-scoped OIDC to the reusable scheduler, and preserve the product boundary in which the sandbox returns artifact-analysis evidence while hosts retain WAF/IDS, admission, final verdict, incident, and retention authority. - Reject `.github/` and `scripts/ci/` from review-thread-derived autofix path authority so an untrusted inline reviewer cannot authorize the write-capable repair agent to modify workflows, CODEOWNERS, actions, scheduler code, or CI helpers that govern its own control plane. - Require the model-write snapshot and exact-path allowlist to remain outside the pull-request worktree, checking both absolute and resolved locations so repository-local controls and outside-looking symlinks resolving into the repository fail closed before they can authorize or verify model changes. diff --git a/CLAUDE.md b/CLAUDE.md index 4b32c05c1..80762fd46 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -136,7 +136,8 @@ repeatable compile command. breakout. Do not reintroduce bash fast-path extraction. - **Cloudflare changes are dry-run by default**; nothing is deleted unless `prune = true` is set explicitly. PRs never see the Cloudflare API token. -- **Org-wide binding conventions** (permissive licenses only — verify SPDX before adding anything; +- **Org-wide binding conventions** (the commercial license allowlist permits permissive licenses + plus the explicit weak-copyleft MPL-2.0 exception — verify SPDX before adding anything; cross-repo references as `owner/repo#num` or full URLs; durable knowledge in the repo/Project, not private memory; one roadmap phase at a time) are defined in `docs/CWL-MASTER-CONTEXT.md` §7 and apply here. diff --git a/docs/CWL-MASTER-CONTEXT.md b/docs/CWL-MASTER-CONTEXT.md index 86a944369..a28c781ef 100644 --- a/docs/CWL-MASTER-CONTEXT.md +++ b/docs/CWL-MASTER-CONTEXT.md @@ -101,7 +101,7 @@ Literature-grounded (see papers below). A dyad can hold MULTIPLE relationship ty A **source-agnostic artifact-analysis service**: `submit(artifact, context) → {verdict, confidence, evidence, IOCs}`. Consumers: naruon email/file attachments (quarantine BEFORE store), platform uploads, connector inputs, API, GitHub issue/PR comments (one trigger). WITHOUT VirusTotal (self-contained): static (YARA(BSD) + capa(Apache) capability→ATT&CK + LIEF/pefile + unzip/macro extract + entropy + context heuristics) + dynamic detonation in a gVisor/Firecracker (Apache) microVM with eBPF behavioral monitoring (Falco/Tetragon, Apache) + network sinkhole + **LLM reasoning (via contextual-orchestrator) over the evidence** + KG/IOC correlation (self-hosted growing reputation). Auto-response per consumer (GitHub → delete comment + block user; email → quarantine + flag; upload → reject + notify). Validated by a real incident 2026-07-08 (user mapasevo21 posted a `sarif_bypass_patch.zip` malware lure on .github#365 + naruon#977 — deleted + blocked manually; this is what the SOC would automate). ## 7. Engineering conventions (BINDING, all agents) -- **Commercial/permissive licenses ONLY** — MIT/Apache-2.0/BSD/ISC/MPL-2.0/PostgreSQL. NO GPL/AGPL/copyleft/non-commercial. Verify via `gh api repos// --jq .license.spdx_id` before adding. (ZITADEL=AGPL removed; MinerU=Apache OK; ParadeDB pg_search=AGPL avoid; ClamAV=GPL avoid.) +- **Commercially acceptable license allowlist ONLY** — MIT/Apache-2.0/BSD/ISC/MPL-2.0/PostgreSQL. MPL-2.0 is the sole explicitly accepted weak-copyleft license; NO GPL/AGPL/strong copyleft/non-commercial. Verify via `gh api repos// --jq .license.spdx_id` before adding. (ZITADEL=AGPL removed; MinerU=Apache OK; ParadeDB pg_search=AGPL avoid; ClamAV=GPL avoid.) - **DB object names = 2+ word snake_case** (don't rename existing Camel/Pascal). - **Config/secrets from a KV/credential store, NOT os.getenv** (env only as bootstrap transport). - **Attach relevant paper PDFs in PRs** (permissive redistribution only). diff --git a/docs/doctoring/merge-scheduler-workflow-run-deduplication.md b/docs/doctoring/merge-scheduler-workflow-run-deduplication.md new file mode 100644 index 000000000..c00546648 --- /dev/null +++ b/docs/doctoring/merge-scheduler-workflow-run-deduplication.md @@ -0,0 +1,60 @@ +# Central merge-scheduler `workflow_run` deduplication + +검토 기준일: **2026-08-21** + +## Incident + +The central `Required PR Review Merge Scheduler` accumulated several queued +`workflow_run` executions with the same default-branch `head_sha` and no PR +metadata. These runs were redundant repository-wide scans. The existing +current-head cleanup handled pull-request, push, and schedule runs, but did +not classify this metadata-free `workflow_run` shape. The same event also used +the default-branch fallback without `cancel-in-progress`, so later runs did +not remove an older queued scan. + +## Decision + +1. Include only metadata-free `workflow_run` events in the scheduler's + existing conditional `cancel-in-progress` expression. PR-associated + workflow-run events retain their existing non-cancelling behavior and + PR-specific concurrency key; metadata-free events use the workflow-run- + specific default-branch fallback, separate from push runs. +2. During the organization sweep, inspect only runs named exactly + `Required PR Review Merge Scheduler` with event `workflow_run`, the current + default branch, an empty `pull_requests` list, and complete Actions-run + evidence. +3. Restrict the dedupe to the exact current default-branch `head_sha`, then + sort by creation time and run ID. Keep the newest run and cancel older + same-head runs. If the current default SHA is absent, skip this additional + cancellation path; older-head cleanup remains governed by the existing + stale-run policy. +4. Preserve the existing fail-closed behavior: incomplete PR-head, default + branch, or Actions-run reads disable queue cancellation for that repository. + +This is queue hygiene only. It does not reinterpret a check, publish a status, +approve a pull request, alter branch protection, or merge a branch. + +## Verification and rollback + +The contract test proves the exact workflow trigger, workflow identity, +metadata boundary, same-head selection, and deterministic ordering. The extracted jq +program was also run against a fixture containing duplicate same-head runs, +an older-head run, a PR-associated run, and an unrelated workflow; it selected +only the older duplicate. `actionlint`, `git diff +--check`, and the focused pytest passed on the change branch. + +Rollback reverts the workflow concurrency expression, the additional queue +hygiene block, the focused contract test, and these documentation entries in +one normal pull request. No GitHub Actions registry state is mutated by the +code change itself. + +## References + +GitHub. (2026). *Control the concurrency of workflows and jobs*. GitHub Docs. +https://docs.github.com/en/enterprise-cloud@latest/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency + +GitHub. (2026). *Events that trigger workflows: `workflow_run`*. GitHub Docs. +https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows + +GitHub. (2026). *Managing workflow runs*. GitHub Docs. +https://docs.github.com/en/actions/how-tos/manage-workflow-runs diff --git a/docs/doctoring/required-review-check-is-not-a-verdict.md b/docs/doctoring/required-review-check-is-not-a-verdict.md new file mode 100644 index 000000000..573cae551 --- /dev/null +++ b/docs/doctoring/required-review-check-is-not-a-verdict.md @@ -0,0 +1,109 @@ +# Required OpenCode/Noema checks are not reviews + +검토 기준일: **2026-08-14** + +## Incident + +On ContextualWisdomLab/contextual-orchestrator#176 the required +`opencode-review` and `noema-review` checks were green, but the Reviews +API had no APPROVE or REQUEST_CHANGES. Authors treated the check name as +a review verdict (GitHub, n.d.-a). That is weaker than the modern-review +expectation that a review is an explicit, current-head judgment +(Bacchelli & Bird, 2013). + +## Decision + +The required `opencode-review` job on +`.github/workflows/opencode-review.yml` never runs the model. Privileged +review stays in `opencode-review-dispatch.yml`. The required job now +reads current-head reviews with `pull-requests: read` and **fails +closed** unless `opencode-agent` / `opencode-agent[bot]` already posted +`APPROVED` or `CHANGES_REQUESTED` on that SHA. A COMMENTED review, a +review on an old SHA, or no review at all cannot make the check green. + +`scripts/ci/noema_review_gate.py` no longer returns 0 when the current +head has no primary OpenCode approval. That skip was exit 0, so the +required `noema-review` check looked like a successful review. Draft +status is checked only after that primary-approval gate, so a draft +without an OpenCode verdict cannot turn `noema-review` green. The gate +also validates the primary approval before accepting an existing Noema +review; a secondary verdict cannot independently turn the required gate +green. + +Human `repository_dispatch` as `seonghobae` remains rejected; only +`github-actions[bot]` may start the privileged dispatch. After a real +verdict is posted, re-run the required `opencode-review` job so the +fail-closed check can observe it. + +The one-dispatch-per-run budget used to walk pull requests in created-at +order, so leftover increments that already had a previous-head verdict +consumed the slot while a later PR with an empty Reviews tab waited. The +scheduler now stable-sorts that budget: no OpenCode APPROVED or +CHANGES_REQUESTED on any commit first, then previous-head re-reviews, +then current-head verdicts. COMMENTED-only evidence is not a verdict and +keeps the empty-Reviews priority. + +A second same-head `repository_dispatch` used to cancel the first through +workflow `cancel-in-progress` because `active_review_run_refs` compared +the GitHub `name` field to the short alias `OpenCode Review Dispatch`. +Live runs set `name` to the interpolated run-name. The matcher now +accepts that prefix so a queued or in-progress same-head review is +`already_running`. + +## Exact CodeQL merge-preview contract + +The central CodeQL pull-request workflow must analyze the merge tree formed by +the current base and head, not merely trust the event's +`pull_request.merge_commit_sha`. A live CodeQL failure showed that GitHub can +provide a merge commit whose parents and tree belong to an older pull-request +head. The workflow now verifies the supplied identity, computes +`git merge-tree --write-tree` for the current pair, and materializes a +deterministic two-parent local commit when the supplied metadata is stale or +structurally different. CodeQL receives the resulting analyzed SHA and the +workflow records both identities, so a stale preview cannot silently become +current-head evidence (GitHub, n.d.-b). + +## Draft pull-request review contract + +Draft status is a merge-readiness signal, not a request to suppress early +feedback. The central scheduler therefore dispatches same-head Strix first +and then authenticated OpenCode review for draft pull requests. The draft +path is deliberately review-only: it cannot update the head branch, enable +or disable auto-merge, merge, dismiss reviews, or resolve review threads. +Marking a pull request ready remains the explicit boundary for merge +automation. + +## Verification contract + +- `tests/test_opencode_required_verdict_gate.py` pins + `current_head_opencode_verdict` and `decide_required_verdict_check`. +- `tests/test_noema_review_gate.py` requires exit 1 when there is no + primary OpenCode approval, even when the current head already has a Noema + review, while retaining the idempotent success path after a valid primary + approval exists. +- `tests/test_opencode_agent_contract.py` pins the required workflow + fail-closed error string. +- `tests/test_pr_review_merge_scheduler.py` proves that a draft pull request + receives same-head Strix and OpenCode dispatch while branch updates, + auto-merge mutation, direct merge, review dismissal, and thread cleanup + remain unreachable, and that a never-reviewed pull request consumes the + one-dispatch budget before a leftover increment that already has a + previous-head OpenCode verdict. +- Both repairs were exercised test-first: the draft-dispatch contract failed + against the old unconditional skip, and the Noema ordering contract failed + against the old secondary-review-first branch. The exact repaired source + then passed 988 tests, 7,056 production statements, 2,834 production + branches, and the public-docstring gate at 100%. + +## References (APA 7th) + +Bacchelli, A., & Bird, C. (2013). Expectations, outcomes, and challenges of +modern code review. In *Proceedings of the 35th International Conference on +Software Engineering* (pp. 712–721). IEEE. +https://doi.org/10.1109/ICSE.2013.6606617 + +GitHub. (n.d.-a). *About status checks*. GitHub Docs. Retrieved +August 14, 2026, from https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks + +GitHub. (n.d.-b). *About code scanning with CodeQL*. GitHub Docs. Retrieved +August 22, 2026, from https://docs.github.com/en/code-security/code-scanning/introduction-to-code-scanning/about-code-scanning-with-codeql diff --git a/docs/org-required-workflow-rollout.md b/docs/org-required-workflow-rollout.md index 3d2e1ac61..669a1387f 100644 --- a/docs/org-required-workflow-rollout.md +++ b/docs/org-required-workflow-rollout.md @@ -1,6 +1,6 @@ # ContextualWisdomLab central required workflow rollout -Updated: 2026-08-22 KST +Updated: 2026-08-24 KST ## Decision @@ -89,9 +89,13 @@ code scanning analyses for ruleset `18156473` `code_scanning` (CodeQL, Scorecard osv-scanner). They trigger on pull requests to `main`, `master`, and `develop` so Git Flow repositories on `develop` inherit the same merge gate as GitHub Flow repos. -CodeQL merge preview checks out `refs/pull//merge` and uploads SARIF with -`sha: pull_request.merge_commit_sha` because the ruleset evaluates that commit, -not the ephemeral merge ref OID. +CodeQL merge preview verifies the current base/head pair and computes its exact +merge tree before analysis. GitHub's `pull_request.merge_commit_sha` remains +the supplied preview identity, but if its parents or tree are stale the +workflow materializes a deterministic local two-parent merge preview and +publishes the SHA of the tree it actually analyzed. The documented +`refs/pull//merge` value remains only the SARIF ref label; the analyzed +merge SHA and its tree are the authoritative publication identity. Repository-local `codeql.yml` push/default-branch scans may remain for branch history, but PR merge gates should rely on the central `codeql-pr.yml` workflow. diff --git a/docs/pr-review-and-merge-procedure.md b/docs/pr-review-and-merge-procedure.md index 8da4703f3..b4222fb47 100644 --- a/docs/pr-review-and-merge-procedure.md +++ b/docs/pr-review-and-merge-procedure.md @@ -69,6 +69,14 @@ block a policy-clean current head. The scheduler logs the exact run id and bounded API error as an Actions warning, then continues the current-head decision. +Metadata-free `workflow_run` scheduler events use a workflow-run-specific +default-branch fallback concurrency group, separate from push runs. They +therefore cancel older runs in that group, and the organization sweep keeps +only the newest same-HEAD central scheduler scan. This cleanup is limited to +the exact scheduler workflow, the default branch, and an empty +`pull_requests` list; push, PR-associated, and unrelated workflow runs remain +eligible for their own queue decisions. + Old approvals and old checks are not merge evidence after the head SHA changes. OpenCode review evidence must be internally same-head as well as GitHub-attached same-head. If the review body includes `Gate evidence` with diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index 9317860e4..01ab88268 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -16,6 +16,7 @@ import urllib.parse import urllib.request from collections.abc import Sequence +from pathlib import Path from typing import Any @@ -380,13 +381,38 @@ def review_thread_context(pr: dict[str, Any]) -> str: return "\n".join(lines) +def codegraph_context_root() -> Path: + """Return the workspace root that may contain CodeGraph context files.""" + raw = os.environ.get("GITHUB_WORKSPACE", "").strip() or os.getcwd() + return Path(raw).resolve() + + +def confined_codegraph_context_path(path: str, root: Path) -> Path | None: + """Return the resolved path when it cannot escape the workspace root.""" + candidate = Path(path) + if ".." in candidate.parts: + return None + if not candidate.is_absolute(): + candidate = Path(root / candidate) + try: + resolved = candidate.resolve() + except OSError: + return None + if not resolved.is_relative_to(root): + return None + return resolved + + def load_codegraph_context() -> str: """Load optional precomputed CodeGraph context for structural review evidence.""" path = os.environ.get("NOEMA_CODEGRAPH_CONTEXT_PATH", "").strip() if not path: return "" + confined = confined_codegraph_context_path(path, codegraph_context_root()) + if confined is None: + return "CodeGraph context unavailable: path escapes the workspace." try: - with open(path, encoding="utf-8") as handle: + with confined.open(encoding="utf-8") as handle: return truncate_text(handle.read(), MAX_REVIEW_CONTEXT_CHARS) except OSError as exc: return f"CodeGraph context unavailable: {exc}" @@ -582,7 +608,12 @@ def submit_review(repo: str, number: int, pr: dict[str, Any], actor: str, verdic def inspect_and_review(repo: str, number: int) -> int: - """Inspect PR state and submit Noema's LLM review when gates are clean.""" + """Inspect PR state and submit Noema's LLM review when gates are clean. + + Missing current-head primary OpenCode approval fails closed, including + on draft pull requests, so the required check cannot look reviewed + without a Reviews-tab verdict. + """ pr = fetch_pr(repo, number) actor = current_actor() if actor in PRIMARY_REVIEW_AUTHORS: @@ -591,15 +622,19 @@ def inspect_and_review(repo: str, number: int) -> int: "Noema review skipped so GitHub receives an independent reviewer." ) return 0 + if not current_primary_approval(pr): + print( + "Current head does not have a primary OpenCode approval; " + "Noema cannot skip as success because that made the required " + "check look like a review." + ) + return 1 if pr.get("isDraft"): - print("PR is draft; Noema review skipped.") + print("PR is draft; Noema review skipped after primary OpenCode approval.") return 0 if existing_noema_review(pr, actor): print("Current head already has a Noema review; nothing to do.") return 0 - if not current_primary_approval(pr): - print("Current head does not have a primary OpenCode approval; Noema review skipped.") - return 0 if has_current_changes_requested(pr): print("Current head has requested changes; Noema review skipped.") return 0 diff --git a/scripts/ci/opencode_dispatch_status.py b/scripts/ci/opencode_dispatch_status.py index 9109a0248..9224361ff 100644 --- a/scripts/ci/opencode_dispatch_status.py +++ b/scripts/ci/opencode_dispatch_status.py @@ -10,16 +10,74 @@ try: from opencode_existing_approval_gate import ( + FALLBACK_MARKERS, OPENCODE_APP_APPROVAL_AUTHORS, review_rejection_reason, ) except ModuleNotFoundError: # pragma: no cover - package import path from scripts.ci.opencode_existing_approval_gate import ( + FALLBACK_MARKERS, OPENCODE_APP_APPROVAL_AUTHORS, review_rejection_reason, ) +OPENCODE_VERDICT_STATES = frozenset({"APPROVED", "CHANGES_REQUESTED"}) +MISSING_VERDICT_MESSAGE = ( + "No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head. " + "This required check is not a review and must not succeed until the " + "authenticated dispatch posts a current-head verdict." +) + + +def current_head_opencode_verdict( + reviews: Sequence[dict[str, Any]], head_sha: str +) -> str | None: + """Return the latest substantive current-head OpenCode verdict, if any.""" + expected = (head_sha or "").lower() + if not expected: + return None + for review in reversed(reviews): + author = str((review.get("user") or {}).get("login") or "").casefold() + if author not in OPENCODE_APP_APPROVAL_AUTHORS: + continue + if str(review.get("commit_id") or "").lower() != expected: + continue + state = str(review.get("state") or "").upper() + if state not in OPENCODE_VERDICT_STATES: + return None + body = str(review.get("body") or "").casefold() + if state == "APPROVED" and any(marker in body for marker in FALLBACK_MARKERS): + return None + return state + return None + + +def decide_required_verdict_check( + *, + expected_head: str, + pull_request: dict[str, Any], + reviews: Sequence[dict[str, Any]], +) -> dict[str, str]: + """Fail closed unless OpenCode already published a current-head verdict.""" + live_head = str((pull_request.get("head") or {}).get("sha") or "") + if not expected_head or live_head.lower() != expected_head.lower(): + return { + "state": "failure", + "description": ( + "OpenCode required-check target is stale or the live PR head " + "is unavailable." + ), + } + verdict = current_head_opencode_verdict(reviews, expected_head) + if verdict is None: + return {"state": "failure", "description": MISSING_VERDICT_MESSAGE} + return { + "state": "success", + "description": f"Current-head OpenCode verdict: {verdict}.", + } + + def _has_current_approval(reviews: Sequence[dict[str, Any]], head_sha: str) -> bool: """Return whether the latest OpenCode decision is a verified approval.""" for review in reversed(reviews): @@ -67,10 +125,15 @@ def decide_status( def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: - """Parse commit-status evidence inputs.""" + """Parse commit-status or required-verdict evidence inputs.""" parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--model-outcome", required=True) - parser.add_argument("--coverage-result", required=True) + parser.add_argument( + "--mode", + choices=("dispatch-status", "required-verdict"), + default="dispatch-status", + ) + parser.add_argument("--model-outcome") + parser.add_argument("--coverage-result") parser.add_argument("--expected-head", required=True) parser.add_argument("--pull-request-file", required=True, type=Path) parser.add_argument("--reviews-file", required=True, type=Path) @@ -78,12 +141,22 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: def main(argv: Sequence[str] | None = None) -> int: - """Print one JSON commit-status decision.""" + """Print one JSON decision and exit 1 when the required verdict is missing.""" args = parse_args(argv) pull_request = json.loads(args.pull_request_file.read_text(encoding="utf-8")) reviews = json.loads(args.reviews_file.read_text(encoding="utf-8")) if not isinstance(pull_request, dict) or not isinstance(reviews, list): raise SystemExit("pull request evidence must be an object and reviews evidence an array") + if args.mode == "required-verdict": + decision = decide_required_verdict_check( + expected_head=args.expected_head, + pull_request=pull_request, + reviews=reviews, + ) + print(json.dumps(decision, separators=(",", ":"))) + return 0 if decision["state"] == "success" else 1 + if not args.model_outcome or not args.coverage_result: + raise SystemExit("--model-outcome and --coverage-result are required") print( json.dumps( decide_status( @@ -99,5 +172,5 @@ def main(argv: Sequence[str] | None = None) -> int: return 0 -if __name__ == "__main__": +if __name__ == "__main__": # pragma: no cover - exercised through main() raise SystemExit(main()) diff --git a/scripts/ci/pr_review_merge_scheduler.py b/scripts/ci/pr_review_merge_scheduler.py index 44620fcab..7dd40ba13 100644 --- a/scripts/ci/pr_review_merge_scheduler.py +++ b/scripts/ci/pr_review_merge_scheduler.py @@ -1277,6 +1277,38 @@ def has_current_head_changes_requested(pr: dict[str, Any]) -> bool: return current_head_review_state(pr, "CHANGES_REQUESTED") +def has_any_opencode_verdict(pr: dict[str, Any]) -> bool: + """Return whether OpenCode ever posted APPROVED or CHANGES_REQUESTED on this PR.""" + for review in (pr.get("reviews") or {}).get("nodes") or []: + if not is_opencode_review(review): + continue + if is_deterministic_fallback_approval(review): + continue + if (review.get("state") or "").upper() in {"APPROVED", "CHANGES_REQUESTED"}: + return True + return False + + +def review_dispatch_priority(pr: dict[str, Any]) -> int: + """Return a lower rank for PRs that should consume the dispatch budget first. + + Rank 0 has no OpenCode APPROVED or CHANGES_REQUESTED on any commit — the + empty Reviews tab from ContextualWisdomLab/contextual-orchestrator#176. + Rank 1 already received a verdict on a previous head and can wait for a + leftover re-review. Rank 2 already has a current-head verdict. + """ + if has_current_head_approval(pr) or has_current_head_changes_requested(pr): + return 2 + if has_any_opencode_verdict(pr): + return 1 + return 0 + + +def prioritize_review_dispatch_queue(prs: Sequence[dict[str, Any]]) -> list[dict[str, Any]]: + """Stable-sort so never-reviewed PRs take the dispatch slot before leftover increments.""" + return sorted(prs, key=review_dispatch_priority) + + def stale_opencode_change_request_ids(pr: dict[str, Any]) -> list[int]: """Return dismissible automated change requests tied to previous heads.""" review_ids: list[int] = [] @@ -1979,6 +2011,22 @@ def stale_opencode_run_ids(repo: str, workflow: str, pr: dict[str, Any]) -> list return stale +def workflow_run_name_matches( + run_name: str, workflow: str, workflow_aliases: frozenset[str] +) -> bool: + """Return whether a GitHub run name is the workflow or a run-name prefix of it. + + Workflow runs use ``run-name:`` as ``name``. OpenCode Review Dispatch therefore + appears as ``OpenCode Review Dispatch owner/repo#1@sha``, which is not equal to + the short alias. Exact-only matching misses the live run, a second dispatch is + posted, and ``cancel-in-progress`` kills the review that was about to finish. + """ + names = {workflow, *workflow_aliases} + if run_name in names: + return True + return any(run_name.startswith(f"{candidate} ") for candidate in names) + + def active_review_run_refs( repo: str, workflow: str, @@ -2010,13 +2058,27 @@ def active_review_run_refs( for run_repo in (dispatch_repo,): for run_data in active_workflow_runs(run_repo, statuses): run_name = str(run_data.get("name") or "") - if run_name != workflow and run_name not in workflow_aliases: + display_title = str(run_data.get("display_title") or "") + # Required-workflow pull_request_target runs materialize the protected + # check only; they never execute the authenticated reviewer. Ignore + # that placeholder even in the legacy same-repository mode so it + # cannot suppress the real repository_dispatch review. + if run_data.get("event") == "pull_request_target" and ( + run_name == run_title + or display_title == run_title + or display_title.startswith(f"{run_title} ") + ): + continue + if not workflow_run_name_matches( + run_name, workflow, workflow_aliases + ) and not workflow_run_name_matches( + display_title, workflow, workflow_aliases + ): continue run_id = run_data.get("id") if not run_id: continue run_ref = (run_repo, str(run_id)) - display_title = str(run_data.get("display_title") or "") dispatch_title_prefix = next( ( prefix @@ -2333,6 +2395,125 @@ def current_head_can_attempt_merge(pr: dict[str, Any], merge_state: str) -> bool return False + +def inspect_draft_pr_for_review( + repo: str, + pr: dict[str, Any], + *, + dry_run: bool, + trigger_reviews: bool, + review_dispatch_allowed: bool, + workflow: str, + security_workflow: str, + stale_opencode_minutes: int, +) -> Decision: + """Dispatch current-head review evidence without mutating a draft PR branch. + + Draft pull requests remain excluded from branch updates, auto-merge, + direct merge, stale-review dismissal, and review-thread mutation. They + still need early Strix and OpenCode feedback so authors can finish the + implementation before marking the pull request ready for review. + """ + number = pr["number"] + if has_current_head_changes_requested(pr): + return Decision( + number, + "skip", + "draft PR; current-head OpenCode review requested changes", + ) + if has_current_head_approval(pr): + return Decision( + number, + "skip", + "draft PR; current-head OpenCode approval recorded", + ) + if not trigger_reviews: + return Decision(number, "skip", "draft PR; review dispatch disabled") + + opencode_state = opencode_progress_state( + pr, + stale_after_minutes=stale_opencode_minutes, + ) + if opencode_state == "running": + return Decision( + number, + "wait", + "draft PR; OpenCode review is already in progress", + ) + if opencode_state == "complete": + return Decision( + number, + "skip", + "draft PR; OpenCode review already completed without a current-head verdict", + ) + if not review_dispatch_allowed: + return Decision(number, "wait", "draft PR; review dispatch limit reached") + + strix_state = strix_evidence_state(pr) + if strix_state == "missing": + wait_reason = repository_dispatch_wait_reason(repo, security_workflow) + if wait_reason: + return Decision( + number, + "wait", + "draft PR; current head has no completed Strix evidence; " + f"{wait_reason}", + ) + strix_dispatch_result = dispatch_strix_evidence( + repo, + security_workflow, + pr, + dry_run=dry_run, + ) + if strix_dispatch_result == "already_running": + return Decision( + number, + "wait", + "draft PR; current head has no completed Strix evidence; " + "same-head Strix workflow run is already active", + ) + return Decision( + number, + "security_dispatch", + "draft PR; current head has no completed Strix evidence; " + "same-head Strix dispatched", + ) + if strix_state == "running": + return Decision( + number, + "wait", + "draft PR; same-head Strix evidence is still running", + ) + + wait_reason = repository_dispatch_wait_reason(repo, workflow) + if wait_reason: + return Decision( + number, + "wait", + "draft PR; current head has completed Strix evidence; " + f"{wait_reason}", + ) + dispatch_result = dispatch_opencode_review( + repo, + workflow, + pr, + dry_run=dry_run, + ) + if dispatch_result == "already_running": + return Decision( + number, + "wait", + "draft PR; current head has completed Strix evidence; " + "same-head OpenCode workflow run is already active", + ) + return Decision( + number, + "review_dispatch", + "draft PR; current head has completed Strix evidence; " + "same-head OpenCode dispatched", + ) + + def inspect_pr( repo: str, pr: dict[str, Any], @@ -2355,7 +2536,16 @@ def inspect_pr( base_ref = pr.get("baseRefName") if pr.get("isDraft"): - return Decision(number, "skip", "draft PR") + return inspect_draft_pr_for_review( + repo, + pr, + dry_run=dry_run, + trigger_reviews=trigger_reviews, + review_dispatch_allowed=review_dispatch_allowed, + workflow=workflow, + security_workflow=security_workflow, + stale_opencode_minutes=stale_opencode_minutes, + ) cancel_stale_pr_runs(repo, pr, dry_run=dry_run) if base_ref != base_branch: # Stacked/cascade PR (base is another feature branch). Org required @@ -3918,6 +4108,8 @@ def main(argv: list[str]) -> int: 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 not args.pr_number: + prs = prioritize_review_dispatch_queue(prs) decisions = [] review_dispatches_used = 0 branch_updates_used = 0 diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index 3d3449dae..bb4bb27fe 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -526,6 +526,7 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$bootstrap_file" "coverage-evidence:" "opencode required workflow preserves the stable coverage-evidence branch-protection context" assert_file_contains "$bootstrap_file" "name: opencode-review" "opencode required workflow preserves the stable opencode-review branch-protection context" assert_file_contains "$bootstrap_file" "authenticated default-branch OpenCode review dispatch" "opencode required workflow delegates real review execution to the protected dispatch path" + assert_file_contains "$bootstrap_file" "This required check is not a review" "opencode required workflow fails closed without a current-head OpenCode verdict" assert_file_not_contains "$bootstrap_file" "repository_dispatch:" "opencode required workflow does not mix privileged dispatch execution with pull_request_target" assert_file_not_contains "$bootstrap_file" "actions/checkout" "opencode required workflow never checks out pull-request content" assert_file_not_contains "$bootstrap_file" '${{ secrets.' "opencode required workflow never binds repository secrets" @@ -953,7 +954,10 @@ assert_file_contains "$REPO_ROOT/scripts/ci/run_opencode_review_model_pool.sh" ' if [[ "$coverage_merge_tree_step" != *'GH_TOKEN: ${{ steps.coverage_read_app_token.outputs.token || secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }}'* ]]; then record_failure "opencode coverage merge-tree fetch must use the coverage App token and central fallback credentials before github.token for target repository reads" fi - assert_file_contains "$workflow_file" 'fetch --no-tags --prune --no-recurse-submodules origin "$PR_BASE_SHA" "$PR_HEAD_SHA"' "coverage evidence fetches exact base and head commits as data" + assert_file_contains "$workflow_file" 'fetch --no-tags --prune --no-recurse-submodules origin "$PR_BASE_SHA"' "coverage evidence fetches the exact base commit as data" + assert_file_contains "$workflow_file" 'fetch --no-tags --prune --no-recurse-submodules origin "$PR_HEAD_SHA"' "coverage evidence first attempts the exact head commit as data" + assert_file_contains "$workflow_file" 'refs/pull/${PR_NUMBER}/head:refs/remotes/origin/pr-${PR_NUMBER}-head' "coverage evidence can resolve an external exact head through the target PR ref" + assert_file_contains "$workflow_file" 'fetched_head_sha="$(git -C "$fetch_dir" rev-parse "refs/remotes/origin/pr-${PR_NUMBER}-head")"' "coverage evidence binds the fetched PR ref back to the expected exact head" assert_file_contains "$workflow_file" 'merge --no-ff --no-edit "$PR_HEAD_SHA"' "coverage evidence materializes the current pull request merge tree without action checkout" assert_file_contains "$workflow_file" "Coverage merge tree could not be materialized" "coverage evidence logs an actionable merge-tree failure reason" assert_file_contains "$workflow_file" "--require-hashes" "coverage tooling installs from a hash-pinned lock" @@ -1542,6 +1546,8 @@ assert_pr_review_merge_scheduler_uses_github_actions_bot_token() { assert_file_contains "$workflow_file" "github.event_name == 'schedule' && format('schedule-{0}', github.event.schedule)" "scheduler isolates the 15-minute organization sweep from the separate 30-minute scheduled scan" assert_file_contains "$workflow_file" "github.event_name == 'repository_dispatch' && github.event.client_payload.target_repository != '' && github.event.client_payload.pr_number != ''" "scheduler scopes targeted manual queue scans to the requested PR" assert_file_contains "$workflow_file" "cancel-in-progress: \${{ github.event_name == 'pull_request_target' || github.event_name == 'pull_request_review' || github.event_name == 'repository_dispatch' || (github.event_name == 'workflow_run' && !github.event.workflow_run.pull_requests[0].number) }}" "scheduler cancels stale PR/review/manual queue scans instead of accumulating merge/update attempts" + assert_file_contains "$workflow_file" "github.event_name == 'repository_dispatch' && format('repo-dispatch-{0}', github.repository)" "scheduler keeps manual queue scans isolated per repository" + assert_file_contains "$workflow_file" "github.event_name == 'workflow_run' && !github.event.workflow_run.pull_requests[0].number" "scheduler cancels only metadata-free workflow-run scans in their isolated fallback group" assert_file_contains "$workflow_file" "timeout-minutes: 60" "organization sweep has enough headroom to finish the complete repository walk" assert_file_contains "$workflow_file" "ORG_SWEEP_TRIGGER_REVIEWS: \${{ github.event_name == 'schedule' ||" "scheduled organization sweeps retry missing current-head OpenCode reviews" assert_file_contains "$workflow_file" "ORG_SWEEP_ENABLE_AUTO_MERGE: \${{ github.event_name == 'schedule' ||" "scheduled organization sweeps merge approved current heads" diff --git a/scripts/ci/validate_vcs_dependency_license.py b/scripts/ci/validate_vcs_dependency_license.py new file mode 100755 index 000000000..69a16285c --- /dev/null +++ b/scripts/ci/validate_vcs_dependency_license.py @@ -0,0 +1,119 @@ +#!/usr/bin/env python3 +"""Fail closed unless one exact organization VCS revision has a permitted license.""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +import urllib.request +from typing import Any + + +ORGANIZATION = "ContextualWisdomLab" +REPOSITORY_RE = re.compile(r"^[A-Za-z0-9_.-]{1,100}$") +COMMIT_RE = re.compile(r"^[0-9a-f]{40}$") +MAX_METADATA_BYTES = 1024 * 1024 +REQUEST_TIMEOUT_SECONDS = 30 +PERMITTED_SPDX_IDS = frozenset( + { + "MIT", + "Apache-2.0", + "BSD-2-Clause", + "BSD-3-Clause", + "ISC", + "MPL-2.0", + "PostgreSQL", + } +) + + +class RejectRedirectHandler(urllib.request.HTTPRedirectHandler): + """Reject redirects before urllib can contact their destination.""" + + def redirect_request( + self, + req: Any, + fp: Any, + code: int, + msg: str, + headers: Any, + newurl: str, + ) -> None: + del req, fp, code, msg, headers, newurl + raise RuntimeError("VCS dependency license metadata redirect is forbidden") + + +def _license_url(repository: str, commit: str) -> str: + """Return the fixed-origin GitHub license URL for one exact revision.""" + if REPOSITORY_RE.fullmatch(repository) is None or repository in {".", ".."}: + raise ValueError("VCS dependency repository is malformed") + if COMMIT_RE.fullmatch(commit) is None: + raise ValueError("VCS dependency commit is not an exact lowercase SHA") + return ( + f"https://api.github.com/repos/{ORGANIZATION}/{repository}/" + f"license?ref={commit}" + ) + + +def _default_opener() -> urllib.request.OpenerDirector: + """Build a no-proxy opener for the fixed public GitHub API request.""" + return urllib.request.build_opener( + urllib.request.ProxyHandler({}), + RejectRedirectHandler(), + ) + + +def validate_license( + repository: str, + commit: str, + *, + opener: Any | None = None, +) -> str: + """Return the permitted SPDX ID or fail closed on any metadata ambiguity.""" + url = _license_url(repository, commit) + request = urllib.request.Request( + url, + headers={ + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + "User-Agent": "ContextualWisdomLab-opencode-vcs-license-gate", + }, + ) + client = opener if opener is not None else _default_opener() + with client.open(request, timeout=REQUEST_TIMEOUT_SECONDS) as response: + if response.geturl() != url: + raise RuntimeError("VCS dependency license metadata left the fixed GitHub origin") + payload_bytes = response.read(MAX_METADATA_BYTES + 1) + if len(payload_bytes) > MAX_METADATA_BYTES: + raise RuntimeError("VCS dependency license metadata exceeded the size limit") + try: + payload = json.loads(payload_bytes.decode("utf-8", errors="strict")) + spdx_id = payload["license"]["spdx_id"] + except (KeyError, TypeError, UnicodeDecodeError, json.JSONDecodeError) as exc: + raise ValueError("VCS dependency license metadata is malformed") from exc + if not isinstance(spdx_id, str) or spdx_id not in PERMITTED_SPDX_IDS: + raise ValueError( + f"VCS dependency SPDX license {spdx_id!r} is not permitted" + ) + return spdx_id + + +def main(argv: list[str] | None = None) -> int: + """Validate CLI arguments and print only the authoritative SPDX identifier.""" + parser = argparse.ArgumentParser() + parser.add_argument("--repository", required=True) + parser.add_argument("--commit", required=True) + args = parser.parse_args(argv) + try: + spdx_id = validate_license(args.repository, args.commit) + except (OSError, RuntimeError, ValueError) as exc: + print(f"VCS dependency license validation failed: {exc}", file=sys.stderr) + return 1 + print(spdx_id) + return 0 + + +if __name__ == "__main__": # pragma: no cover - exercised by the CLI boundary. + raise SystemExit(main()) diff --git a/tests/test_codeql_pr_workflow_contract.py b/tests/test_codeql_pr_workflow_contract.py index 813385b23..d29604a99 100644 --- a/tests/test_codeql_pr_workflow_contract.py +++ b/tests/test_codeql_pr_workflow_contract.py @@ -35,8 +35,38 @@ def test_codeql_pr_workflow_gates_head_and_merge_sarif_locally() -> None: assert "CodeQL merge preview" in workflow assert "github.event.pull_request.head.sha" in workflow assert "github.event.pull_request.merge_commit_sha" in workflow + assert "name: Verify merge preview identity" in workflow + assert "id: verify-merge-preview" in workflow + assert 'git rev-list --parents -n 1 "$MERGE_SHA"' in workflow + assert 'git merge-tree --write-tree "$BASE_SHA" "$HEAD_SHA"' in workflow + assert 'git rev-parse "$MERGE_SHA^{tree}"' in workflow + assert 'GITHUB_TOKEN: ${{ github.token }}' in workflow + assert 'GIT_CONFIG_VALUE_0="AUTHORIZATION: bearer $GITHUB_TOKEN"' in workflow + assert 'git fetch --no-tags origin "$BASE_SHA" "$MERGE_SHA"' in workflow + assert 'git fetch --no-tags origin "$BASE_SHA" "$HEAD_SHA" "$MERGE_SHA"' not in workflow + assert 'git cat-file -e "$HEAD_SHA^{commit}"' in workflow + assert 'fetch --no-tags origin "$HEAD_SHA"' in workflow + assert ( + '"+refs/pull/${PR_NUMBER}/head:refs/remotes/origin/' + 'pr-${PR_NUMBER}-head"' + ) in workflow + assert ( + 'fetched_head_sha="$(git rev-parse ' + '"refs/remotes/origin/pr-${PR_NUMBER}-head")"' + ) in workflow + assert '[ "$fetched_head_sha" = "$HEAD_SHA" ]' in workflow + assert "CodeQL merge preview could not resolve exact head SHA" in workflow + assert 'git commit-tree "$expected_tree" -p "$BASE_SHA" -p "$HEAD_SHA"' in workflow + assert 'git reset --hard "$local_merge_sha"' in workflow + assert 'echo "merge_sha=$local_merge_sha"' in workflow + assert "sha: ${{ steps.verify-merge-preview.outputs.merge_sha }}" in workflow assert "refs/pull/{0}/head" in workflow - assert "refs/pull/{0}/merge" in workflow + assert "ref: ${{ github.event.pull_request.merge_commit_sha }}" in workflow + merge_checkout = workflow.split(" - name: Checkout merge preview", 1)[1].split( + " - name: Verify merge preview identity", 1 + )[0] + assert "fetch-depth: 0" in merge_checkout + assert "ref: ${{ format('refs/pull/{0}/merge', github.event.pull_request.number) }}" in workflow assert workflow.count("security-events: read") == 2 assert "security-events: write" not in workflow diff --git a/tests/test_noema_review_gate.py b/tests/test_noema_review_gate.py index 408bb95b9..be96a377f 100644 --- a/tests/test_noema_review_gate.py +++ b/tests/test_noema_review_gate.py @@ -258,6 +258,7 @@ def fake_run(args, stdin=None): monkeypatch.setattr(noema, "run", fake_run) codegraph_path = tmp_path / "codegraph.md" codegraph_path.write_text("call graph: src/a.py -> tests", encoding="utf-8") + monkeypatch.setenv("GITHUB_WORKSPACE", str(tmp_path)) monkeypatch.setenv("NOEMA_CODEGRAPH_CONTEXT_PATH", str(codegraph_path)) pr = make_pr( headRefOid="head sha", @@ -294,6 +295,7 @@ def fake_run(args, stdin=None): def test_review_context_reports_omitted_files_and_missing_codegraph(monkeypatch, tmp_path): + monkeypatch.setenv("GITHUB_WORKSPACE", str(tmp_path)) monkeypatch.delenv("NOEMA_CODEGRAPH_CONTEXT_PATH", raising=False) assert noema.load_codegraph_context() == "" @@ -309,6 +311,64 @@ def test_review_context_reports_omitted_files_and_missing_codegraph(monkeypatch, assert "1 changed files omitted from context budget" in context +def test_load_codegraph_context_rejects_workspace_escape(monkeypatch, tmp_path): + """Traversal, absolute, and symlink paths outside the workspace are rejected.""" + monkeypatch.setenv("GITHUB_WORKSPACE", str(tmp_path)) + outside = tmp_path.parent / "passwd-shape" + outside.write_text("root:x:0:0:root:/root:/bin/sh\n", encoding="utf-8") + monkeypatch.setenv("NOEMA_CODEGRAPH_CONTEXT_PATH", str(outside)) + assert noema.load_codegraph_context() == ( + "CodeGraph context unavailable: path escapes the workspace." + ) + + monkeypatch.setenv( + "NOEMA_CODEGRAPH_CONTEXT_PATH", + str(tmp_path / "nested" / ".." / ".." / outside.name), + ) + assert "path escapes the workspace" in noema.load_codegraph_context() + + link = tmp_path / "escape.md" + link.symlink_to(outside) + monkeypatch.setenv("NOEMA_CODEGRAPH_CONTEXT_PATH", str(link)) + assert "path escapes the workspace" in noema.load_codegraph_context() + + +def test_codegraph_context_root_and_resolve_failure(monkeypatch, tmp_path): + """Workspace root prefers GITHUB_WORKSPACE and resolve errors stay closed.""" + monkeypatch.setenv("GITHUB_WORKSPACE", str(tmp_path)) + assert noema.codegraph_context_root() == tmp_path.resolve() + monkeypatch.delenv("GITHUB_WORKSPACE", raising=False) + monkeypatch.chdir(tmp_path) + assert noema.codegraph_context_root() == tmp_path.resolve() + + class FailingPath(type(tmp_path)): + """Path stand-in whose resolve always fails.""" + + def resolve(self, *args, **kwargs): + """Raise OSError to cover the confinement resolve failure.""" + raise OSError("resolve failed") + + monkeypatch.setattr(noema, "Path", FailingPath) + assert noema.confined_codegraph_context_path("graph.md", tmp_path.resolve()) is None + + +def test_relative_codegraph_context_resolves_from_workspace(monkeypatch, tmp_path): + """Relative context paths are workspace-relative even when cwd differs.""" + workspace = tmp_path / "workspace" + context_dir = workspace / "context" + context_dir.mkdir(parents=True) + graph = context_dir / "graph.md" + graph.write_text("trusted workspace graph", encoding="utf-8") + cwd = tmp_path / "cwd" + cwd.mkdir() + + monkeypatch.chdir(cwd) + monkeypatch.setenv("GITHUB_WORKSPACE", str(workspace)) + monkeypatch.setenv("NOEMA_CODEGRAPH_CONTEXT_PATH", "context/graph.md") + + assert noema.load_codegraph_context() == "trusted workspace graph" + + class FakeResponse: """Small context-manager response for urllib monkeypatches.""" @@ -526,10 +586,17 @@ def test_inspect_and_review_skip_paths(monkeypatch): assert noema.inspect_and_review("owner/repo", 7) == 0 assert calls + calls.clear() + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr()) + assert noema.inspect_and_review("owner/repo", 7) == 1 + assert calls == [] + + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: make_pr(isDraft=True)) + assert noema.inspect_and_review("owner/repo", 7) == 1 + assert calls == [] + cases = [ - (make_pr(), "noema"), - (make_pr(isDraft=True), "noema"), - (make_pr(reviews={"nodes": [review(login="noema", body="")]}), "noema"), + (make_pr(isDraft=True, reviews={"nodes": [review(body=marker_body)]}), "noema"), (make_pr(reviews={"nodes": [review("CHANGES_REQUESTED"), review(body=marker_body)]}), "noema"), (make_pr(reviews={"nodes": [review(body=marker_body)]}, reviewThreads={"nodes": [{"isResolved": False, "isOutdated": False}]}), "noema"), (make_pr(reviews={"nodes": [review(body=marker_body)]}, statusCheckRollup={"contexts": {"nodes": [{"__typename": "StatusContext", "context": "ci", "state": "FAILURE"}]}}), "noema"), @@ -543,6 +610,37 @@ def test_inspect_and_review_skip_paths(monkeypatch): assert calls == [] + +def test_existing_noema_review_cannot_bypass_primary_approval(monkeypatch): + """A Noema verdict is never sufficient without current-head OpenCode approval.""" + noema_review = review( + login="noema", + body="", + ) + pr = make_pr(reviews={"nodes": [noema_review]}) + submitted = [] + monkeypatch.setattr(noema, "fetch_pr", lambda repo, number: pr) + monkeypatch.setattr(noema, "current_actor", lambda: "noema") + monkeypatch.setattr( + noema, + "submit_review", + lambda *args, **kwargs: submitted.append(args), + ) + + assert noema.inspect_and_review("owner/repo", 7) == 1 + assert submitted == [] + + primary_review = review( + body=( + "OpenCode reviewed the current-head bounded evidence and found " + "no blocking issues." + ) + ) + pr = make_pr(reviews={"nodes": [primary_review, noema_review]}) + assert noema.inspect_and_review("owner/repo", 7) == 0 + assert submitted == [] + + def test_parse_args_and_main(monkeypatch): parsed = noema.parse_args(["--repo", "owner/repo", "--pr-number", "9"]) assert parsed.repo == "owner/repo" diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index f0b1af470..d75f6b6b3 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -523,10 +523,13 @@ def test_opencode_target_coverage_materializes_only_after_authorized_dispatch(): assert 'http."${GITHUB_SERVER_URL}/".extraheader' not in step assert "AUTHORIZATION: bearer ${GH_TOKEN}" not in step assert "AUTHORIZATION: bearer" not in step + assert 'fetch --no-tags --prune --no-recurse-submodules origin "$PR_BASE_SHA"' in step + assert 'fetch --no-tags --prune --no-recurse-submodules origin "$PR_HEAD_SHA"' in step + assert 'refs/pull/${PR_NUMBER}/head:refs/remotes/origin/pr-${PR_NUMBER}-head' in step assert ( - 'fetch --no-tags --prune --no-recurse-submodules origin "$PR_BASE_SHA" "$PR_HEAD_SHA"' - in step - ) + 'fetched_head_sha="$(git -C "$fetch_dir" rev-parse ' + '"refs/remotes/origin/pr-${PR_NUMBER}-head")"' + ) in step assert "Coverage fetch could not authenticate" in step assert 'merge --no-ff --no-edit "$PR_HEAD_SHA"' in step assert "Coverage merge tree could not be materialized" in step @@ -621,6 +624,20 @@ def test_opencode_target_coverage_materializes_only_after_authorized_dispatch(): assert npm_install_case.count("return 0") == 2 assert "return 1" not in npm_install_case assert "trusted_pnpm_lock_matches_base()" in measure_step + assert "trusted_pnpm_package_manager_matches_base()" in measure_step + assert 'relative_manifest="${relative_dir:+${relative_dir}/}package.json"' in measure_step + assert ( + 'base_spec="$(trusted_git show "${PR_BASE_SHA}:${relative_manifest}" ' + "| jq -er '.packageManager // empty')\"" + in measure_step + ) + assert ( + 'head_spec="$(trusted_git show "${PR_HEAD_SHA}:${relative_manifest}" ' + "| jq -er '.packageManager // empty')\"" + in measure_step + ) + assert "Current pnpm packageManager specification differs from the validated base" in measure_step + assert "export COREPACK_ENABLE_NETWORK=0" in measure_step assert ( 'base_blob="$(trusted_git rev-parse "${PR_BASE_SHA}:${relative_lock}"' in measure_step @@ -668,7 +685,9 @@ def test_opencode_target_coverage_materializes_only_after_authorized_dispatch(): assert "The networked build context contains only this" in measure_step assert 'install -m 0644 "$trusted_ci_requirements"' in measure_step assert 'install -m 0755 "$trusted_base_python_installer"' in measure_step + assert 'install -m 0755 "$trusted_vcs_license_validator"' in measure_step assert "COPY install-base-python-locks.py" in measure_step + assert "COPY validate-vcs-dependency-license.py" in measure_step assert "python3 -I /usr/local/libexec/install-base-python-locks.py" in measure_step assert '"https://github.com/ContextualWisdomLab/${repository}.git"' in measure_step assert '--quiet --no-tags --depth=1 origin "$commit"' in measure_step @@ -677,6 +696,16 @@ def test_opencode_target_coverage_materializes_only_after_authorized_dispatch(): assert "opencode-base-vcs-dependencies.pth" in measure_step assert 'vcs-manifest.json >"$dependency_list"' in measure_step assert 'done <"$dependency_list"' in measure_step + license_validation = measure_step.index( + 'python3 -I /usr/local/libexec/validate-vcs-dependency-license.py' + ) + vcs_clone = measure_step.index('git init --quiet "$destination"') + vcs_registration = measure_step.index( + 'printf \'%s\\n\' "$python_root" >>"$path_file"' + ) + assert license_validation < vcs_clone < vcs_registration + assert '--repository "$repository" --commit "$commit"' in measure_step + assert "Validated permitted VCS dependency license" in measure_step assert 'candidate_count=$((candidate_count + 1))' in measure_step assert '[ "$candidate_count" -ne 1 ]' in measure_step assert "has a missing or ambiguous import root" in measure_step @@ -1182,6 +1211,79 @@ def test_opencode_coverage_gates_trust_lockfile_on_pnpm_11_3(tmp_path): ) +def test_opencode_coverage_rejects_pnpm_package_manager_version_drift(tmp_path): + """A head cannot select a different Corepack pnpm version than the base.""" + bash = shutil.which("bash") + git = shutil.which("git") + jq = shutil.which("jq") + if bash is None or git is None or jq is None: + pytest.skip("bash, git, and jq are required for this workflow regression") + + repo = tmp_path / "repo" + repo.mkdir() + subprocess.run([git, "init", "-q", str(repo)], check=True) + subprocess.run([git, "-C", str(repo), "config", "user.name", "Test"], check=True) + subprocess.run( + [git, "-C", str(repo), "config", "user.email", "test@example.invalid"], + check=True, + ) + (repo / "package.json").write_text( + '{"packageManager":"pnpm@9.15.9"}\n', encoding="utf-8" + ) + (repo / "pnpm-lock.yaml").write_text("lockfileVersion: '9.0'\n", encoding="utf-8") + subprocess.run([git, "-C", str(repo), "add", "."], check=True) + subprocess.run([git, "-C", str(repo), "commit", "-qm", "base"], check=True) + base_sha = subprocess.run( + [git, "-C", str(repo), "rev-parse", "HEAD"], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + (repo / "package.json").write_text( + '{"packageManager":"pnpm@10.28.1"}\n', encoding="utf-8" + ) + subprocess.run([git, "-C", str(repo), "add", "package.json"], check=True) + subprocess.run([git, "-C", str(repo), "commit", "-qm", "head"], check=True) + head_sha = subprocess.run( + [git, "-C", str(repo), "rev-parse", "HEAD"], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + workflow = Path(".github/workflows/opencode-review-dispatch.yml").read_text( + encoding="utf-8" + ) + measure_start = workflow.index(" - name: Measure test and docstring evidence\n") + measure_end = workflow.index("\n - name:", measure_start + 1) + measure_step = workflow[measure_start:measure_end] + helper_start = measure_step.index( + " trusted_pnpm_package_manager_matches_base() {\n" + ) + helper_end = measure_step.index("\n\n trusted_pnpm_lock_matches_base()", helper_start) + helper = textwrap.dedent(measure_step[helper_start:helper_end]) + script = f""" +set -euo pipefail +trusted_git() {{ git -C "$TEST_REPO" "$@"; }} +export COVERAGE_SOURCE_WORKDIR="$TEST_REPO" +export PR_BASE_SHA="{base_sha}" +export PR_HEAD_SHA="{head_sha}" +cd "$TEST_REPO" +{helper} +trusted_pnpm_package_manager_matches_base +""" + completed = subprocess.run( + [bash, "-c", script], + env={**os.environ, "TEST_REPO": str(repo)}, + check=False, + capture_output=True, + text=True, + timeout=10, + ) + assert completed.returncode != 0 + assert "differs from the validated base" in completed.stderr + + def test_opencode_coverage_discovers_changed_nested_javascript_package(tmp_path): """A changed JS file must select its nearest nested package.json for coverage.""" bash = shutil.which("bash") @@ -2416,12 +2518,16 @@ def test_opencode_privileged_review_security_boundaries_are_fail_closed(): assert "repository_dispatch:" not in bootstrap.split("permissions:", 1)[0] assert "actions/checkout" not in bootstrap assert "${{ secrets." not in bootstrap + assert 'jq -r -s --arg sha "$HEAD_SHA"' in bootstrap assert "required-workflow-bootstrap:" in bootstrap assert " coverage-source-tree:\n" in bootstrap assert " coverage-evidence:\n" in bootstrap assert " opencode-review-target:\n" in bootstrap assert " name: opencode-review\n" in bootstrap assert "authenticated default-branch OpenCode review dispatch" in bootstrap + assert "This required check is not a review" in bootstrap + assert "No APPROVED or CHANGES_REQUESTED from opencode-agent on the current head" in bootstrap + assert "pull-requests: read" in bootstrap assert workflow.count("ref: ${{ steps.trusted_source.outputs.ref }}") == 1 assert "TRUSTED_SOURCE_REF: ${{ steps.trusted_source.outputs.ref }}" in workflow assert "ref: ${{ github.workflow_sha }}" not in workflow diff --git a/tests/test_opencode_required_verdict_gate.py b/tests/test_opencode_required_verdict_gate.py new file mode 100644 index 000000000..06431accc --- /dev/null +++ b/tests/test_opencode_required_verdict_gate.py @@ -0,0 +1,246 @@ +"""Fail-closed required OpenCode check when no current-head verdict exists.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from scripts.ci import opencode_dispatch_status as dispatch_status + + +def _review( + *, login: str, state: str, commit_id: str, body: str = "" +) -> dict[str, object]: + """Return one GitHub Reviews API object.""" + return { + "user": {"login": login}, + "state": state, + "commit_id": commit_id, + "body": body, + } + + +def test_current_head_opencode_verdict_reads_latest_matching_state() -> None: + head = "a" * 40 + assert ( + dispatch_status.current_head_opencode_verdict( + [ + _review(login="opencode-agent[bot]", state="APPROVED", commit_id=head), + _review( + login="opencode-agent[bot]", + state="CHANGES_REQUESTED", + commit_id=head, + ), + ], + head, + ) + == "CHANGES_REQUESTED" + ) + assert ( + dispatch_status.current_head_opencode_verdict( + [_review(login="opencode-agent", state="APPROVED", commit_id=head)], + head, + ) + == "APPROVED" + ) + + +def test_current_head_opencode_verdict_ignores_other_actors_and_heads() -> None: + head = "a" * 40 + assert ( + dispatch_status.current_head_opencode_verdict( + [_review(login="coderabbitai[bot]", state="APPROVED", commit_id=head)], + head, + ) + is None + ) + assert ( + dispatch_status.current_head_opencode_verdict( + [ + _review( + login="opencode-agent[bot]", + state="APPROVED", + commit_id="b" * 40, + ) + ], + head, + ) + is None + ) + assert ( + dispatch_status.current_head_opencode_verdict( + [ + _review(login="coderabbitai[bot]", state="APPROVED", commit_id=head), + _review(login="opencode-agent[bot]", state="APPROVED", commit_id="b" * 40), + _review(login="opencode-agent[bot]", state="COMMENTED", commit_id=head), + ], + head, + ) + is None + ) + assert dispatch_status.current_head_opencode_verdict([], "") is None + + +@pytest.mark.parametrize( + "marker", + ( + "deterministic current-head evidence", + "deterministic fallback approval", + "model-unavailable evidence fallback", + "did not emit a usable current-head control block", + "scope: `unsupported`", + "model-pool outcome: `unknown`", + ), +) +def test_current_head_opencode_verdict_rejects_fallback_approval(marker: str) -> None: + """Model-unavailable or deterministic approvals are not formal evidence.""" + head = "a" * 40 + reviews = [ + _review(login="opencode-agent[bot]", state="APPROVED", commit_id=head), + _review( + login="opencode-agent[bot]", + state="APPROVED", + commit_id=head, + body=f"OpenCode {marker}", + ), + ] + + assert dispatch_status.current_head_opencode_verdict(reviews, head) is None + + reviews[-1]["state"] = "CHANGES_REQUESTED" + assert ( + dispatch_status.current_head_opencode_verdict(reviews, head) + == "CHANGES_REQUESTED" + ) + + +def test_current_head_opencode_verdict_uses_latest_current_head_review() -> None: + """A later non-verdict cannot expose an older decision as the latest one.""" + head = "a" * 40 + assert ( + dispatch_status.current_head_opencode_verdict( + [ + _review( + login="opencode-agent[bot]", + state="APPROVED", + commit_id=head, + ), + _review( + login="opencode-agent[bot]", + state="COMMENTED", + commit_id=head, + ), + ], + head, + ) + is None + ) + + +def test_required_workflow_rejects_fallback_approvals() -> None: + """The checkout-free jq twin enforces the same fallback boundary.""" + workflow = Path(".github/workflows/opencode-review.yml").read_text( + encoding="utf-8" + ) + + assert "| (last // {}) as $review" in workflow + for marker in ( + "deterministic current-head evidence", + "deterministic fallback approval", + "model-unavailable evidence fallback", + "did not emit a usable current-head control block", + "scope: `unsupported`", + "model-pool outcome: `unknown`", + ): + assert marker in workflow + + +def test_decide_required_verdict_check_fails_closed_without_verdict() -> None: + head = "a" * 40 + decision = dispatch_status.decide_required_verdict_check( + expected_head=head, + pull_request={"head": {"sha": head}}, + reviews=[], + ) + assert decision["state"] == "failure" + assert "This required check is not a review" in decision["description"] + stale = dispatch_status.decide_required_verdict_check( + expected_head=head, + pull_request={"head": {"sha": "c" * 40}}, + reviews=[_review(login="opencode-agent[bot]", state="APPROVED", commit_id=head)], + ) + assert stale["state"] == "failure" + approved = dispatch_status.decide_required_verdict_check( + expected_head=head, + pull_request={"head": {"sha": head}}, + reviews=[_review(login="opencode-agent[bot]", state="APPROVED", commit_id=head)], + ) + assert approved == { + "state": "success", + "description": "Current-head OpenCode verdict: APPROVED.", + } + + +def test_required_verdict_cli_exits_one_without_verdict(tmp_path: Path) -> None: + head = "a" * 40 + pr_file = tmp_path / "pr.json" + reviews_file = tmp_path / "reviews.json" + pr_file.write_text(json.dumps({"head": {"sha": head}}), encoding="utf-8") + reviews_file.write_text("[]", encoding="utf-8") + assert ( + dispatch_status.main( + [ + "--mode", + "required-verdict", + "--expected-head", + head, + "--pull-request-file", + str(pr_file), + "--reviews-file", + str(reviews_file), + ] + ) + == 1 + ) + reviews_file.write_text( + json.dumps( + [_review(login="opencode-agent[bot]", state="CHANGES_REQUESTED", commit_id=head)] + ), + encoding="utf-8", + ) + assert ( + dispatch_status.main( + [ + "--mode", + "required-verdict", + "--expected-head", + head, + "--pull-request-file", + str(pr_file), + "--reviews-file", + str(reviews_file), + ] + ) + == 0 + ) + + +def test_dispatch_status_cli_still_requires_model_and_coverage(tmp_path: Path) -> None: + head = "a" * 40 + pr_file = tmp_path / "pr.json" + reviews_file = tmp_path / "reviews.json" + pr_file.write_text(json.dumps({"head": {"sha": head}}), encoding="utf-8") + reviews_file.write_text("[]", encoding="utf-8") + with pytest.raises(SystemExit, match="--model-outcome"): + dispatch_status.main( + [ + "--expected-head", + head, + "--pull-request-file", + str(pr_file), + "--reviews-file", + str(reviews_file), + ] + ) diff --git a/tests/test_opencode_workflow_shell_syntax.py b/tests/test_opencode_workflow_shell_syntax.py index 056b450f5..9ead4c9a7 100644 --- a/tests/test_opencode_workflow_shell_syntax.py +++ b/tests/test_opencode_workflow_shell_syntax.py @@ -34,6 +34,11 @@ def test_opencode_review_run_blocks_are_valid_bash(): ) assert 'gsub("`"; "'")' in workflow_text assert 'gsub("`"; "\'")' not in workflow_text + assert ( + ' elif [ "$pr_head_fetch_attempt" -lt 6 ]; then\n' + ' echo "PR head ref fetch failed on attempt $pr_head_fetch_attempt; retrying after propagation delay."\n' + ' sleep 10' + ) in workflow_text if sys.platform == "win32": return @@ -170,8 +175,11 @@ def test_merge_scheduler_targeted_dispatch_validates_live_exact_pr(tmp_path): """#!/usr/bin/env bash set -euo pipefail test "$1" = api -test "$2" = repos/ContextualWisdomLab/naruon/pulls/1179 -printf '%s\\n' "$FAKE_PULL_JSON" +case "$2" in + repos/ContextualWisdomLab/naruon/pulls/1179) printf '%s\\n' "$FAKE_PULL_JSON" ;; + repos/ContextualWisdomLab/naruon) printf '%s\\n' "$FAKE_REPOSITORY_JSON" ;; + *) exit 1 ;; +esac """, encoding="utf-8", ) @@ -193,6 +201,7 @@ def test_merge_scheduler_targeted_dispatch_validates_live_exact_pr(tmp_path): **os.environ, "PATH": f"{fake_bin}:{os.environ['PATH']}", "FAKE_PULL_JSON": json.dumps(pull), + "FAKE_REPOSITORY_JSON": json.dumps({"default_branch": "main"}), "GITHUB_EVENT_NAME": "repository_dispatch", "GITHUB_REPOSITORY": "ContextualWisdomLab/.github", "GITHUB_OUTPUT": str(output), @@ -218,6 +227,7 @@ def test_merge_scheduler_targeted_dispatch_validates_live_exact_pr(tmp_path): assert output.read_text(encoding="utf-8").splitlines() == [ "repository=ContextualWisdomLab/naruon", "base_branch=develop", + "default_branch=main", "head_sha=4afd4af7ad343660356791873d940aa2846f40c2", ] @@ -264,6 +274,7 @@ def test_merge_scheduler_targeted_dispatch_validates_live_exact_pr(tmp_path): assert output.read_text(encoding="utf-8").splitlines() == [ "repository=ContextualWisdomLab/naruon", "base_branch=develop", + "default_branch=main", "head_sha=4afd4af7ad343660356791873d940aa2846f40c2", ] @@ -290,5 +301,87 @@ def test_merge_scheduler_targeted_dispatch_validates_live_exact_pr(tmp_path): ) assert malformed_head.returncode == 1 - assert "malformed live PR metadata" in malformed_head.stdout + assert "malformed, or base-repository-mismatched live PR metadata" in malformed_head.stdout assert not output.exists() + + +def test_opencode_dispatch_validation_accepts_exact_external_head(tmp_path): + """A canonical fork remains exact-head review data, never workflow source.""" + if sys.platform == "win32": + return + bash = shutil.which("bash") + if bash is None: + return + + workflow_text = ( + REPO_ROOT / ".github/workflows/opencode-review-dispatch.yml" + ).read_text(encoding="utf-8") + script = _extract_run_block( + workflow_text, + "Bind workflow inputs to live organization pull request metadata", + ) + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + fake_gh = fake_bin / "gh" + fake_gh.write_text( + """#!/usr/bin/env bash +set -euo pipefail +test "$1" = api +test "$2" = repos/ContextualWisdomLab/naruon/pulls/1179 +printf '%s\\n' "$FAKE_PULL_JSON" +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + pull = { + "number": 1179, + "state": "open", + "base": { + "ref": "develop", + "sha": "1" * 40, + "repo": {"full_name": "ContextualWisdomLab/naruon", "private": False}, + }, + "head": { + "ref": "feature/fork-review", + "sha": "2" * 40, + "repo": {"full_name": "outside/fork"}, + }, + } + output = tmp_path / "github-output" + env = { + **os.environ, + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "FAKE_PULL_JSON": json.dumps(pull), + "EVENT_NAME": "repository_dispatch", + "DISPATCH_ACTOR": "scheduler", + "DISPATCH_SENDER": "scheduler", + "ALLOWED_DISPATCH_ACTOR": "scheduler", + "ALLOWED_DISPATCH_TARGETS": "ContextualWisdomLab/naruon", + "TARGET_REPOSITORY": "ContextualWisdomLab/naruon", + "PR_NUMBER": "1179", + "SUPPLIED_BASE_REF": "develop", + "SUPPLIED_BASE_SHA": "1" * 40, + "SUPPLIED_HEAD_REF": "feature/fork-review", + "SUPPLIED_HEAD_SHA": "2" * 40, + "GITHUB_OUTPUT": str(output), + } + + result = subprocess.run( + [bash], + input=script, + text=True, + capture_output=True, + check=False, + env=env, + ) + + assert result.returncode == 0, result.stderr + assert output.read_text(encoding="utf-8").splitlines() == [ + "target_repository=ContextualWisdomLab/naruon", + "pr_number=1179", + "base_ref=develop", + f"base_sha={'1' * 40}", + "head_ref=feature/fork-review", + f"head_sha={'2' * 40}", + "is_private=false", + ] diff --git a/tests/test_pr_review_autofix_nvidia_nim_contract.py b/tests/test_pr_review_autofix_nvidia_nim_contract.py index a5d25379a..15ed86b44 100644 --- a/tests/test_pr_review_autofix_nvidia_nim_contract.py +++ b/tests/test_pr_review_autofix_nvidia_nim_contract.py @@ -19,7 +19,7 @@ DOCTORING_RECORD = Path("docs/doctoring/hourly-nvidia-nim-autofix.md") CHANGELOG = Path("CHANGELOG.md") REVIEW_DISPATCH_WORKFLOW = Path(".github/workflows/opencode-review-dispatch.yml") -REVIEW_DISPATCH_BLOB_SHA = "0df7a17cc72a79585cec169c8299e0646f93ab02" +REVIEW_DISPATCH_BLOB_SHA = "978845efeddee88404ce6be5a53fcdd0d80436ed" def _workflow_text(path: Path) -> str: @@ -62,6 +62,7 @@ def test_scheduled_autofix_uses_only_nvidia_nim() -> None: '"apiKey": "{env:STRIX_GITHUB_MODELS_TOKEN}"', '"baseURL": "https://models.github.ai/inference"', 'COPILOT_GITHUB_TOKEN', + 'CLOUDFLARE_API_TOKEN', ) for fragment in forbidden_fragments: assert fragment not in workflow, fragment @@ -155,8 +156,38 @@ def test_missing_nvidia_nim_secret_fails_closed_before_model_execution() -> None assert guard in workflow[conflict_start:] +def test_independent_review_agent_key_system_is_unchanged() -> None: + """Pin reviewer write credentials without freezing unrelated workflow bytes.""" + workflow = _workflow_text(REVIEW_DISPATCH_WORKFLOW) + for expression in ( + "GH_TOKEN: $" + + "{{ secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }}", + "GH_TOKEN: $" + "{{ secrets.OPENCODE_APPROVE_TOKEN || github.token }}", + "GH_TOKEN: $" + + "{{ steps.opencode_app_token.outputs.token || secrets.PR_REVIEW_MERGE_TOKEN || secrets.OPENCODE_APPROVE_TOKEN || github.token }}", + ): + assert expression in workflow + assert "pr-review-autofix" not in workflow + assert "COPILOT_GITHUB_TOKEN" not in workflow + + model_step_start = workflow.index(" - name: Run OpenCode PR Review model pool") + model_step_end = workflow.index( + " - name: Publish OpenCode review outcome", model_step_start + ) + model_step = workflow[model_step_start:model_step_end] + for provider_key in ( + "STRIX_GITHUB_MODELS_TOKEN", + "NVIDIA_API_KEY", + "OPENROUTER_API_KEY", + "OPENAI_API_KEY", + ): + assert provider_key in model_step + assert "PR_REVIEW_MERGE_TOKEN" not in model_step + assert "OPENCODE_APPROVE_TOKEN" not in model_step + + def test_independent_review_agent_workflow_matches_reviewed_blob() -> None: - """Pin the reviewed read-only reviewer workflow byte-for-byte.""" + """Pin the complete reviewed reviewer workflow in addition to semantic guards.""" result = subprocess.run( ["git", "hash-object", str(REVIEW_DISPATCH_WORKFLOW)], check=True, @@ -164,7 +195,6 @@ def test_independent_review_agent_workflow_matches_reviewed_blob() -> None: text=True, ) assert result.stdout.strip() == REVIEW_DISPATCH_BLOB_SHA - assert "pr-review-autofix" not in _workflow_text(REVIEW_DISPATCH_WORKFLOW) def test_ordinary_autofix_uses_the_same_exact_write_scope_as_conflict_repair() -> None: @@ -233,6 +263,12 @@ def test_operator_doctoring_and_changelog_record_exact_write_scope() -> None: assert "OpenCode. (2026a). *Permissions*" in doctoring assert "ignored-path inventory" in changelog assert "model-mutable Git metadata" in changelog + assert "Allowed an allowlisted base repository's open fork-head PR" in changelog + assert ( + "external fork heads now fail closed before OIDC, review-token, CodeGraph, " + "model, or merge-control paths" + not in changelog + ) def test_allowed_path_seal_accepts_the_structured_inventory(tmp_path: Path) -> None: diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index 0e71bdbe2..d6f0e3a74 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -2294,6 +2294,77 @@ def fake_run(args, stdin=None): assert not any(call[:3] == ["gh", "workflow", "run"] for call in calls) +def test_workflow_run_name_matches_accepts_github_run_name_prefix(): + aliases = frozenset({"OpenCode Review Dispatch", "Required OpenCode Review"}) + assert sched.workflow_run_name_matches( + "OpenCode Review Dispatch", "OpenCode Review", aliases + ) + assert sched.workflow_run_name_matches( + "OpenCode Review Dispatch ContextualWisdomLab/.github#1002@" + ("a" * 40), + "OpenCode Review", + aliases, + ) + assert not sched.workflow_run_name_matches( + "Strix Security Scan ContextualWisdomLab/.github#1002@" + ("a" * 40), + "OpenCode Review", + aliases, + ) + + +def test_dispatch_opencode_review_deduplicates_github_run_name_as_display_title( + monkeypatch, capsys +): + calls = [] + head_sha = "a" * 40 + live_name = f"OpenCode Review Dispatch owner/repo#1@{head_sha}" + current_dispatch = { + "id": 9101, + "name": live_name, + "event": "repository_dispatch", + "head_sha": "default-branch-sha", + "display_title": live_name, + "pull_requests": [], + } + + def fake_run(args, stdin=None): + calls.append(args) + if args[:5] == [ + "gh", + "api", + "--method", + "GET", + "repos/ContextualWisdomLab/.github/actions/runs", + ]: + if "status=queued" in args: + return json.dumps({"workflow_runs": [current_dispatch]}) + return json.dumps({"workflow_runs": []}) + if "/actions/runs" in " ".join(args): + return json.dumps({"workflow_runs": []}) + return "" + + monkeypatch.setattr(sched, "run", fake_run) + monkeypatch.setenv("GITHUB_ACTIONS", "true") + monkeypatch.setenv("GH_TOKEN", "workflow-token") + monkeypatch.setenv( + "SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", + "ContextualWisdomLab/.github", + ) + + result = sched.dispatch_opencode_review( + "owner/repo", + "OpenCode Review", + make_pr(headRefOid=head_sha), + dry_run=False, + ) + + assert result == "already_running" + assert ( + "active same-head workflow run(s) ContextualWisdomLab/.github@9101" + in capsys.readouterr().out + ) + assert not any(call[-2:] == ["--input", "-"] for call in calls) + + def test_dispatch_strix_cancels_stale_central_run_and_keeps_current(monkeypatch, capsys): calls = [] head_sha = "a" * 40 @@ -2382,6 +2453,13 @@ def test_central_run_filter_ignores_malformed_and_non_dispatch_titles(monkeypatc "head_sha": head_sha, "pull_requests": [{"number": 1}], }, + { + "id": 9405, + "name": "Required OpenCode Review", + "event": "workflow_run", + "head_sha": head_sha, + "pull_requests": [], + }, ] def fake_active_runs(repo, statuses=("queued", "in_progress")): @@ -2461,6 +2539,34 @@ def test_central_run_filter_ignores_same_repository_required_workflow_placeholde ) == ([], []) +def test_legacy_same_repository_filter_ignores_required_workflow_placeholder( + monkeypatch, +): + """A required-workflow placeholder must not suppress the real dispatch.""" + head_sha = "a" * 40 + placeholder = { + "id": 9404, + "name": "Required OpenCode Review", + "event": "pull_request_target", + "head_sha": head_sha, + "display_title": f"Required OpenCode Review owner/repo#1@{head_sha}", + "pull_requests": [{"number": 1}], + } + + monkeypatch.setattr( + sched, + "active_workflow_runs", + lambda repo, statuses=("queued", "in_progress"): [placeholder], + ) + monkeypatch.delenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", raising=False) + + assert sched.active_opencode_run_refs( + "owner/repo", + "OpenCode Review", + make_pr(headRefOid=head_sha), + ) == ([], []) + + def test_active_run_filters_and_stale_opencode_dry_run(monkeypatch): runs = [ { @@ -2993,8 +3099,216 @@ def test_summary_section_helpers_handle_empty_and_action_error_cases(): assert "- PR #5: `fork/repo` is external" in "\n".join(external_merge_lines) +def test_draft_pr_review_path_never_mutates_branch_or_merge_state(monkeypatch): + mutations = [] + dispatches = [] + for name in ("update_branch", "enable_auto_merge", "merge_pr", "disable_auto_merge"): + monkeypatch.setattr( + sched, + name, + lambda *args, _name=name, **kwargs: mutations.append(_name), + ) + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda *args, **kwargs: "", + ) + monkeypatch.setattr( + sched, + "dispatch_strix_evidence", + lambda repo, workflow, pr, dry_run: dispatches.append( + ("strix", pr["number"], dry_run) + ) + or "dispatched", + ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatches.append( + ("opencode", pr["number"], dry_run) + ) + or "dispatched", + ) + + missing_strix = inspect(make_pr(isDraft=True), dry_run=False) + assert missing_strix.action == "security_dispatch" + assert missing_strix.reason == ( + "draft PR; current head has no completed Strix evidence; same-head Strix dispatched" + ) + + completed_strix = inspect( + make_pr( + isDraft=True, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ), + dry_run=False, + ) + assert completed_strix.action == "review_dispatch" + assert completed_strix.reason == ( + "draft PR; current head has completed Strix evidence; same-head OpenCode dispatched" + ) + + approved = inspect( + make_pr( + isDraft=True, + autoMergeRequest={"enabledAt": "now"}, + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ), + dry_run=False, + ) + assert approved.action == "skip" + assert approved.reason == "draft PR; current-head OpenCode approval recorded" + + changes_requested = inspect( + make_pr( + isDraft=True, + autoMergeRequest={"enabledAt": "now"}, + reviews={"nodes": [opencode_review("CHANGES_REQUESTED", "head")]}, + ), + dry_run=False, + ) + assert changes_requested.action == "skip" + assert changes_requested.reason == ( + "draft PR; current-head OpenCode review requested changes" + ) + + assert dispatches == [("strix", 1, False), ("opencode", 1, False)] + assert mutations == [] + + +def test_draft_pr_review_wait_states_are_read_only(monkeypatch): + dispatches = [] + monkeypatch.setattr( + sched, + "dispatch_strix_evidence", + lambda *args, **kwargs: dispatches.append("strix") or "dispatched", + ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: dispatches.append("opencode") or "dispatched", + ) + + disabled = inspect(make_pr(isDraft=True), trigger_reviews=False) + assert disabled.action == "skip" + assert disabled.reason == "draft PR; review dispatch disabled" + + running = inspect( + make_pr( + isDraft=True, + statusCheckRollup={ + "contexts": {"nodes": [opencode_check(status="IN_PROGRESS")]} + }, + ) + ) + assert running.action == "wait" + assert running.reason == "draft PR; OpenCode review is already in progress" + + limited = inspect(make_pr(isDraft=True), review_dispatch_allowed=False) + assert limited.action == "wait" + assert limited.reason == "draft PR; review dispatch limit reached" + + completed = inspect( + make_pr( + isDraft=True, + statusCheckRollup={ + "contexts": { + "nodes": [strix_check(), opencode_check(status="COMPLETED")] + } + }, + ) + ) + assert completed.action == "skip" + assert completed.reason == ( + "draft PR; OpenCode review already completed without a current-head verdict" + ) + + strix_running = inspect( + make_pr( + isDraft=True, + statusCheckRollup={ + "contexts": { + "nodes": [strix_check(status="IN_PROGRESS", conclusion=None)] + } + }, + ) + ) + assert strix_running.action == "wait" + assert strix_running.reason == "draft PR; same-head Strix evidence is still running" + assert dispatches == [] + + +def test_draft_pr_review_dispatch_failures_are_wait_states(monkeypatch): + missing_reason = "central Strix dispatch workflow unavailable" + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda repo, workflow: missing_reason + if workflow == "Strix Security Scan" + else "", + ) + missing = inspect(make_pr(isDraft=True)) + assert missing.action == "wait" + assert missing.reason == ( + "draft PR; current head has no completed Strix evidence; " + missing_reason + ) + + opencode_reason = "central OpenCode dispatch workflow unavailable" + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda repo, workflow: opencode_reason + if workflow == "OpenCode Review" + else "", + ) + completed = make_pr( + isDraft=True, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ) + unavailable = inspect(completed) + assert unavailable.action == "wait" + assert unavailable.reason == ( + "draft PR; current head has completed Strix evidence; " + opencode_reason + ) + + monkeypatch.setattr( + sched, + "repository_dispatch_wait_reason", + lambda *args, **kwargs: "", + ) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda *args, **kwargs: "already_running", + ) + already_running = inspect(completed) + assert already_running.action == "wait" + assert already_running.reason == ( + "draft PR; current head has completed Strix evidence; " + "same-head OpenCode workflow run is already active" + ) + + monkeypatch.setattr( + sched, + "dispatch_strix_evidence", + lambda *args, **kwargs: "already_running", + ) + strix_already_running = inspect(make_pr(isDraft=True)) + assert strix_already_running.action == "wait" + assert strix_already_running.reason == ( + "draft PR; current head has no completed Strix evidence; " + "same-head Strix workflow run is already active" + ) + + def test_inspect_pr_blocks_and_waits_for_policy_states(monkeypatch): - assert inspect(make_pr(isDraft=True)).action == "skip" + monkeypatch.delenv("SCHEDULER_REQUIRED_WORKFLOW_REPOSITORY", raising=False) + draft = inspect(make_pr(isDraft=True)) + assert draft.action == "security_dispatch" + assert draft.reason == ( + "draft PR; current head has no completed Strix evidence; same-head Strix dispatched" + ) stacked = inspect(make_pr(baseRefName="develop")) assert stacked.action == "review_dispatch" assert stacked.reason == "stacked PR onto develop; OpenCode review dispatched" @@ -4483,6 +4797,114 @@ def test_main_limits_review_dispatches_and_branch_updates(monkeypatch, capsys): ) +def test_review_dispatch_priority_ranks_empty_reviews_before_leftover_rereview(): + never_reviewed = make_pr(number=176) + leftover_rereview = make_pr( + number=998, + reviews={ + "nodes": [ + opencode_review("COMMENTED", "old-head", login="seonghobae"), + opencode_review("CHANGES_REQUESTED", "old-head"), + ] + }, + ) + already_verdicted = make_pr( + number=1002, + reviews={"nodes": [opencode_review("APPROVED", "head")]}, + ) + commented_only = make_pr( + number=42, + reviews={"nodes": [opencode_review("COMMENTED", "head")]}, + ) + + human_only = make_pr( + number=7, + reviews={"nodes": [opencode_review("APPROVED", "head", login="seonghobae")]}, + ) + fallback_only = make_pr( + number=8, + reviews={ + "nodes": [ + { + **opencode_review("APPROVED", "old-head"), + "body": "Deterministic fallback approval: providers unavailable.", + } + ] + }, + ) + assert sched.has_any_opencode_verdict(never_reviewed) is False + assert sched.has_any_opencode_verdict(commented_only) is False + assert sched.has_any_opencode_verdict(human_only) is False + assert sched.has_any_opencode_verdict(fallback_only) is False + assert sched.has_any_opencode_verdict(leftover_rereview) is True + assert sched.review_dispatch_priority(never_reviewed) == 0 + assert sched.review_dispatch_priority(commented_only) == 0 + assert sched.review_dispatch_priority(fallback_only) == 0 + assert sched.review_dispatch_priority(leftover_rereview) == 1 + assert sched.review_dispatch_priority(already_verdicted) == 2 + assert [ + pr["number"] + for pr in sched.prioritize_review_dispatch_queue( + [ + leftover_rereview, + already_verdicted, + never_reviewed, + commented_only, + fallback_only, + ] + ) + ] == [176, 42, 8, 998, 1002] + + +def test_main_prefers_never_reviewed_pr_when_dispatch_budget_is_one(monkeypatch, capsys): + prs = [ + make_pr( + number=998, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + reviews={"nodes": [opencode_review("CHANGES_REQUESTED", "old-head")]}, + ), + make_pr( + number=176, + statusCheckRollup={"contexts": {"nodes": [strix_check()]}}, + ), + ] + dispatched = [] + + monkeypatch.setattr(sched, "fetch_open_prs", lambda repo, max_prs: prs) + monkeypatch.setattr( + sched, + "dispatch_opencode_review", + lambda repo, workflow, pr, dry_run: dispatched.append(pr["number"]), + ) + monkeypatch.setattr(sched, "cancel_stale_pr_runs", lambda repo, pr, dry_run: []) + + assert ( + sched.main( + [ + "--repo", + "owner/repo", + "--base-branch", + "main", + "--project-flow", + "github-flow", + "--review-dispatch-limit", + "1", + ] + ) + == 0 + ) + + output = capsys.readouterr().out + payload = json.loads(output.strip().splitlines()[-1]) + assert dispatched == [176] + assert payload["decisions"][0]["pr"] == 176 + assert payload["decisions"][0]["action"] == "review_dispatch" + assert payload["decisions"][1]["pr"] == 998 + assert payload["decisions"][1]["reason"] == ( + "current head has completed Strix evidence; review dispatch limit reached" + ) + + def test_main_rejects_invalid_review_dispatch_limit(): with pytest.raises(SystemExit, match="--review-dispatch-limit must be -1 or greater"): sched.main( diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 1d79f1daa..15c55f60a 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -93,7 +93,7 @@ def test_merge_scheduler_deduplicates_unscoped_repository_dispatches() -> None: assert "format('org-sweep-{0}', github.repository)" in concurrency_contract assert "format('repo-dispatch-{0}', github.repository)" in concurrency_contract - assert "format('workflow-run-no-pr-{0}', github.repository)" in concurrency_contract + assert "format('workflow-run-{0}', github.ref)" in concurrency_contract assert ( "github.event_name == 'workflow_run' && !github.event.workflow_run.pull_requests[0].number" in concurrency_contract @@ -123,6 +123,29 @@ def test_merge_scheduler_provides_same_repository_dispatch_credential() -> None: assert workflow.count("SCHEDULER_DISPATCH_TOKEN: ${{ github.token }}") == 2 +def test_scheduler_deduplicates_metadata_free_workflow_run_scans() -> None: + """Keep only the newest same-head central scheduler scan without PR metadata.""" + workflow = workflow_text("pr-review-merge-scheduler.yml") + concurrency = workflow.split("concurrency:", 1)[1].split( + "permissions:", 1 + )[0] + cancel_line = next( + line for line in concurrency.splitlines() if "cancel-in-progress:" in line + ) + queue_hygiene = workflow.split("# Queue hygiene, part 1:", 1)[1].split( + "# Queue hygiene, part 2:", 1 + )[0] + + assert "github.event_name == 'workflow_run'" in concurrency + assert "github.event_name == 'workflow_run' && !github.event.workflow_run.pull_requests[0].number" in cancel_line + assert "format('workflow-run-{0}', github.ref)" in concurrency + assert '.event == "workflow_run"' in queue_hygiene + assert '.name == "Required PR Review Merge Scheduler"' in queue_hygiene + assert '((.pull_requests // []) | length) == 0' in queue_hygiene + assert '.head_sha == $current_default_sha' in queue_hygiene + assert 'sort_by(.created_at, .id)' in queue_hygiene + + def test_targeted_scheduler_dispatch_is_allowlisted_and_exact_pr_scoped() -> None: """Central single-PR dispatch accepts a bounded fork head without trusting it.""" workflow = workflow_text("pr-review-merge-scheduler.yml") @@ -147,6 +170,9 @@ def test_targeted_scheduler_dispatch_is_allowlisted_and_exact_pr_scoped() -> Non ) in validation assert '[ "$live_head_repository" != "$TARGET_REPOSITORY_INPUT" ]' not in validation assert "Targeted scheduler dispatch base branch does not match the live PR" in validation + assert 'gh api "repos/${TARGET_REPOSITORY_INPUT}"' in validation + assert "live_default_branch=" in validation + assert "printf 'default_branch=%s\\n' \"$live_default_branch\"" in validation assert "TARGET_REPOSITORY: ${{ steps.targeted_dispatch.outputs.repository }}" in inspect assert ( "TARGET_DEFAULT_BRANCH: ${{ steps.targeted_dispatch.outputs.base_branch }}" @@ -169,6 +195,24 @@ def test_targeted_scheduler_dispatch_is_allowlisted_and_exact_pr_scoped() -> Non ) in workflow +def test_fork_heads_are_review_only_in_privileged_paths() -> None: + """Canonical external heads reach exact review but never automated merge.""" + workflow = workflow_text("opencode-review-dispatch.yml") + metadata = workflow_step( + workflow, "Bind workflow inputs to live organization pull request metadata" + ) + privileged = workflow_step(workflow, "Validate pull request head repository trust") + scheduler = (REPO_ROOT / "scripts/ci/pr_review_merge_scheduler.py").read_text( + encoding="utf-8" + ) + + assert '[ "$live_head_repository" != "$TARGET_REPOSITORY" ]' not in metadata + assert '[ "$head_repository" != "$GH_REPOSITORY" ]' not in privileged + assert '! [[ "$head_repository" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]]' in privileged + assert "Validated exact-head OpenCode review source" in privileged + assert "fork or external PR heads are excluded from scheduler direct merge and auto-merge" in scheduler + + def test_privileged_review_retries_use_default_branch_repository_dispatch() -> None: """Privileged retries must never load workflow code from a selected ref.""" expected_types = { diff --git a/tests/test_validate_vcs_dependency_license.py b/tests/test_validate_vcs_dependency_license.py new file mode 100644 index 000000000..d7296045f --- /dev/null +++ b/tests/test_validate_vcs_dependency_license.py @@ -0,0 +1,223 @@ +"""Contract tests for exact-revision VCS dependency license validation.""" + +from __future__ import annotations + +import importlib.util +import io +import json +from pathlib import Path +from types import ModuleType + +import pytest + + +SCRIPT = Path("scripts/ci/validate_vcs_dependency_license.py") +MASTER_CONTEXT = Path("docs/CWL-MASTER-CONTEXT.md") +COMMIT = "61c49c50d3b4a24fc9bd7c6d3a7f2f4ba19d7be6" + + +def load_validator() -> ModuleType: + """Load the production validator only after proving the file exists.""" + assert SCRIPT.is_file(), "the exact-revision VCS license validator is missing" + spec = importlib.util.spec_from_file_location("validate_vcs_dependency_license", SCRIPT) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +class FakeResponse(io.BytesIO): + """Minimal urllib response fixture with a stable final URL.""" + + def __init__(self, payload: dict[str, object], url: str) -> None: + super().__init__(json.dumps(payload).encode("utf-8")) + self._url = url + + def geturl(self) -> str: + """Return the final response URL exposed by urllib.""" + return self._url + + def __enter__(self) -> "FakeResponse": + """Support the response context-manager protocol.""" + return self + + def __exit__(self, *_args: object) -> None: + """Close the in-memory response.""" + self.close() + + +class FakeOpener: + """Capture one outbound license request and return fixture metadata.""" + + def __init__(self, spdx_id: str | None, *, final_url: str | None = None) -> None: + self.spdx_id = spdx_id + self.final_url = final_url + self.request_url = "" + + def open(self, request: object, timeout: int) -> FakeResponse: + """Return one bounded GitHub license response fixture.""" + del timeout + self.request_url = request.full_url # type: ignore[attr-defined] + payload = {"license": {"spdx_id": self.spdx_id}} + return FakeResponse(payload, self.final_url or self.request_url) + + +class RawOpener: + """Return arbitrary response bytes for malformed and oversized fixtures.""" + + def __init__(self, payload: bytes) -> None: + self.payload = payload + + def open(self, request: object, timeout: int) -> FakeResponse: + """Return the raw payload from the otherwise exact request URL.""" + del timeout + response = FakeResponse({}, request.full_url) # type: ignore[attr-defined] + response.seek(0) + response.truncate() + response.write(self.payload) + response.seek(0) + return response + + +@pytest.mark.parametrize( + "spdx_id", + [ + "MIT", + "Apache-2.0", + "BSD-2-Clause", + "BSD-3-Clause", + "ISC", + "MPL-2.0", + "PostgreSQL", + ], +) +def test_permitted_exact_spdx_identifiers_pass(spdx_id: str) -> None: + """Every explicitly governed commercially acceptable SPDX ID passes.""" + validator = load_validator() + opener = FakeOpener(spdx_id) + + assert validator.validate_license("RankWeave", COMMIT, opener=opener) == spdx_id + assert opener.request_url == ( + "https://api.github.com/repos/ContextualWisdomLab/RankWeave/" + f"license?ref={COMMIT}" + ) + + +def test_canonical_policy_distinguishes_mpl_from_strong_copyleft() -> None: + """Canonical prose must not contradict the explicit MPL-2.0 allowlist.""" + policy = MASTER_CONTEXT.read_text(encoding="utf-8") + + assert "MIT/Apache-2.0/BSD/ISC/MPL-2.0/PostgreSQL" in policy + assert "NO GPL/AGPL/strong copyleft/non-commercial" in policy + assert "NO GPL/AGPL/copyleft/non-commercial" not in policy + + +@pytest.mark.parametrize( + "spdx_id", + ["GPL-3.0-only", "AGPL-3.0-or-later", "LGPL-2.1-only", "NOASSERTION", None], +) +def test_disallowed_or_unknown_spdx_identifiers_fail_closed( + spdx_id: str | None, +) -> None: + """Strong-copyleft, unknown, and absent metadata never enter the image.""" + validator = load_validator() + + with pytest.raises(ValueError, match="not permitted"): + validator.validate_license("RankWeave", COMMIT, opener=FakeOpener(spdx_id)) + + +def test_repository_and_commit_are_bounded_before_network_access() -> None: + """Untrusted path syntax cannot steer the fixed GitHub API origin.""" + validator = load_validator() + opener = FakeOpener("MIT") + + for repository in ("../outside", ".", ".."): + with pytest.raises(ValueError, match="repository"): + validator.validate_license(repository, COMMIT, opener=opener) + with pytest.raises(ValueError, match="commit"): + validator.validate_license("RankWeave", "main", opener=opener) + + assert opener.request_url == "" + + +def test_redirected_license_metadata_is_rejected() -> None: + """A redirect cannot substitute a different origin for GitHub metadata.""" + validator = load_validator() + opener = FakeOpener("MIT", final_url="https://attacker.invalid/license.json") + + with pytest.raises(RuntimeError, match="origin"): + validator.validate_license("RankWeave", COMMIT, opener=opener) + + +def test_default_opener_disables_proxy_resolution( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Production requests use the no-proxy opener instead of runner proxy state.""" + validator = load_validator() + opener = FakeOpener("MIT") + captured: list[object] = [] + + def build_opener(*handlers: object) -> FakeOpener: + captured.extend(handlers) + return opener + + monkeypatch.setattr(validator.urllib.request, "build_opener", build_opener) + + assert validator.validate_license("RankWeave", COMMIT) == "MIT" + assert len(captured) == 2 + assert isinstance(captured[0], validator.urllib.request.ProxyHandler) + assert isinstance(captured[1], validator.RejectRedirectHandler) + + +def test_default_opener_rejects_redirect_before_following_target() -> None: + """Production metadata requests never contact a redirect destination.""" + validator = load_validator() + handler = validator.RejectRedirectHandler() + + with pytest.raises(RuntimeError, match="redirect"): + handler.redirect_request( + object(), + object(), + 302, + "Found", + {}, + "https://attacker.invalid/license.json", + ) + + +def test_oversized_and_malformed_metadata_fail_closed() -> None: + """The metadata parser rejects both resource abuse and invalid JSON.""" + validator = load_validator() + + with pytest.raises(RuntimeError, match="size limit"): + validator.validate_license( + "RankWeave", + COMMIT, + opener=RawOpener(b"x" * (validator.MAX_METADATA_BYTES + 1)), + ) + with pytest.raises(ValueError, match="malformed"): + validator.validate_license( + "RankWeave", COMMIT, opener=RawOpener(b"not-json") + ) + + +def test_main_reports_success_and_failure( + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """The CLI prints a permitted ID and converts validation errors to exit 1.""" + validator = load_validator() + arguments = ["--repository", "RankWeave", "--commit", COMMIT] + + monkeypatch.setattr(validator, "validate_license", lambda *_args: "Apache-2.0") + assert validator.main(arguments) == 0 + assert capsys.readouterr().out == "Apache-2.0\n" + + def reject(*_args: object) -> str: + raise ValueError("fixture denial") + + monkeypatch.setattr(validator, "validate_license", reject) + assert validator.main(arguments) == 1 + captured = capsys.readouterr() + assert captured.out == "" + assert "VCS dependency license validation failed: fixture denial" in captured.err