diff --git a/.github/workflows/opencode-review.yml b/.github/workflows/opencode-review.yml index 9e195c2d1..070f48ac9 100644 --- a/.github/workflows/opencode-review.yml +++ b/.github/workflows/opencode-review.yml @@ -255,7 +255,7 @@ jobs: repository: ContextualWisdomLab/.github fetch-depth: 1 persist-credentials: false - ref: ${{ steps.trusted_source.outputs.ref }} + ref: ${{ github.workflow_sha }} - name: Report coverage source materialization failure if: needs.coverage-source-tree.result != 'success' @@ -1375,7 +1375,7 @@ jobs: repository: ContextualWisdomLab/.github fetch-depth: 0 persist-credentials: false - ref: ${{ steps.trusted_source.outputs.ref }} + ref: ${{ github.workflow_sha }} - name: Exchange OpenCode app token for target repository review reads id: review_read_app_token @@ -1605,62 +1605,11 @@ jobs: run: | set -euo pipefail context_env_file="${RUNNER_TEMP:-.}/opencode-review-context.env" - python3 <<'PY' >"$context_env_file" - import json - import os - import re - import sys - - event_path = os.environ.get("GITHUB_EVENT_PATH") - if not event_path: - print("::error::GITHUB_EVENT_PATH is not available for OpenCode review context resolution.", file=sys.stderr) - raise SystemExit(1) - - try: - with open(event_path, encoding="utf-8") as handle: - event = json.load(handle) - except (OSError, json.JSONDecodeError) as exc: - print(f"::error::Could not read GitHub event payload for OpenCode review context: {exc}", file=sys.stderr) - raise SystemExit(1) - - inputs = event.get("inputs") or {} - pull_request = event.get("pull_request") or {} - base = pull_request.get("base") or {} - head = pull_request.get("head") or {} - base_repo = base.get("repo") or {} - values = { - "GH_REPOSITORY": str( - base_repo.get("full_name") or inputs.get("target_repository") or os.environ.get("GITHUB_REPOSITORY") or "" - ).strip(), - "PR_NUMBER": str(pull_request.get("number") or inputs.get("pr_number") or "").strip(), - "PR_BASE_SHA": str(base.get("sha") or inputs.get("pr_base_sha") or "").strip(), - "PR_HEAD_SHA": str(head.get("sha") or inputs.get("pr_head_sha") or "").strip(), - } - values["HEAD_SHA"] = values["PR_HEAD_SHA"] - - validators = { - "GH_REPOSITORY": r"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+", - "PR_NUMBER": r"[1-9][0-9]*", - "PR_BASE_SHA": r"[0-9a-fA-F]{40}", - "PR_HEAD_SHA": r"[0-9a-fA-F]{40}", - "HEAD_SHA": r"[0-9a-fA-F]{40}", - } - for name, pattern in validators.items(): - if not re.fullmatch(pattern, values[name]): - print(f"::error::Invalid OpenCode review context value for {name}.", file=sys.stderr) - raise SystemExit(1) - - for name, value in values.items(): - print(f"{name}={value}") - PY - while IFS='=' read -r name value; do - case "$name" in - GH_REPOSITORY|PR_NUMBER|PR_BASE_SHA|PR_HEAD_SHA|HEAD_SHA) - printf -v "$name" '%s' "$value" - export "$name" - ;; - esac - done <"$context_env_file" + python3 scripts/ci/opencode_review_context.py \ + --event-path "$GITHUB_EVENT_PATH" \ + --env-file "$context_env_file" + # shellcheck source=/dev/null + . "$context_env_file" printf 'Resolved bounded OpenCode review context for %s#%s at %s.\n' \ "$GH_REPOSITORY" "$PR_NUMBER" "$PR_HEAD_SHA" diff --git a/.gitleaksignore b/.gitleaksignore index 0987e9e9c..294118cea 100644 --- a/.gitleaksignore +++ b/.gitleaksignore @@ -1,5 +1,9 @@ # Historical false-positive GitHub-token fixtures from scheduler secret-scrubbing tests. # The live tests now construct these token-like strings at runtime; new findings remain blocking. +995d6b3606e0effe6722c422b369da9ef0171824:tests/test_pr_review_merge_scheduler.py:github-pat:2900 +995d6b3606e0effe6722c422b369da9ef0171824:tests/test_pr_review_merge_scheduler.py:github-pat:2908 +995d6b3606e0effe6722c422b369da9ef0171824:tests/test_pr_review_merge_scheduler.py:github-pat:2923 +995d6b3606e0effe6722c422b369da9ef0171824:tests/test_pr_review_merge_scheduler.py:github-pat:2928 123bddb83426e21367f61affa004f9259b101fe8:tests/test_pr_review_merge_scheduler.py:github-pat:224 123bddb83426e21367f61affa004f9259b101fe8:tests/test_pr_review_merge_scheduler.py:github-pat:227 14655a9304c2dbe1e2ad6c963af4a2a0b7bd4b89:tests/test_pr_review_merge_scheduler.py:github-pat:2770 diff --git a/scripts/ci/opencode_review_context.py b/scripts/ci/opencode_review_context.py new file mode 100644 index 000000000..e9991ae62 --- /dev/null +++ b/scripts/ci/opencode_review_context.py @@ -0,0 +1,92 @@ +"""Resolve bounded OpenCode review context from a GitHub event payload.""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import shlex +import sys +from collections.abc import Mapping, Sequence +from pathlib import Path + + +CONTEXT_VALIDATORS = { + "GH_REPOSITORY": re.compile(r"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+\Z"), + "PR_NUMBER": re.compile(r"[1-9][0-9]*\Z"), + "PR_BASE_SHA": re.compile(r"[0-9a-fA-F]{40}\Z"), + "PR_HEAD_SHA": re.compile(r"[0-9a-fA-F]{40}\Z"), + "HEAD_SHA": re.compile(r"[0-9a-fA-F]{40}\Z"), +} + + +def load_event(path: Path) -> Mapping[str, object]: + """Load a GitHub event payload as a JSON object.""" + try: + event = json.loads(path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError) as exc: + print(f"::error::Could not read GitHub event payload for OpenCode review context: {exc}", file=sys.stderr) + raise SystemExit(1) from exc + if not isinstance(event, dict): + print("::error::GitHub event payload for OpenCode review context was not a JSON object.", file=sys.stderr) + raise SystemExit(1) + return event + + +def object_value(value: object) -> Mapping[str, object]: + """Return object mappings and coerce every other JSON value to an empty object.""" + return value if isinstance(value, dict) else {} + + +def resolve_context(event: Mapping[str, object], default_repository: str) -> dict[str, str]: + """Resolve and validate the OpenCode review context values.""" + inputs = object_value(event.get("inputs")) + pull_request = object_value(event.get("pull_request")) + base = object_value(pull_request.get("base")) + head = object_value(pull_request.get("head")) + base_repo = object_value(base.get("repo")) + values = { + "GH_REPOSITORY": str( + base_repo.get("full_name") or inputs.get("target_repository") or default_repository or "" + ).strip(), + "PR_NUMBER": str(pull_request.get("number") or inputs.get("pr_number") or "").strip(), + "PR_BASE_SHA": str(base.get("sha") or inputs.get("pr_base_sha") or "").strip(), + "PR_HEAD_SHA": str(head.get("sha") or inputs.get("pr_head_sha") or "").strip(), + } + values["HEAD_SHA"] = values["PR_HEAD_SHA"] + for name, pattern in CONTEXT_VALIDATORS.items(): + if not pattern.fullmatch(values[name]): + print(f"::error::Invalid OpenCode review context value for {name}.", file=sys.stderr) + raise SystemExit(1) + return values + + +def write_shell_exports(path: Path, values: Mapping[str, str]) -> None: + """Write validated values as shell export statements.""" + path.write_text( + "".join(f"export {name}={shlex.quote(value)}\n" for name, value in values.items()), + encoding="utf-8", + ) + + +def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace: + """Parse command-line arguments.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--event-path", required=True, type=Path) + parser.add_argument("--env-file", required=True, type=Path) + parser.add_argument("--default-repository", default=os.environ.get("GITHUB_REPOSITORY", "")) + return parser.parse_args(argv) + + +def main(argv: Sequence[str] | None = None) -> int: + """Resolve context files for the OpenCode review workflow.""" + args = parse_args(argv) + event = load_event(args.event_path) + values = resolve_context(event, args.default_repository) + write_shell_exports(args.env_file, values) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/ci/opencode_review_normalize_output.py b/scripts/ci/opencode_review_normalize_output.py index 88b931872..4f18ee7ff 100755 --- a/scripts/ci/opencode_review_normalize_output.py +++ b/scripts/ci/opencode_review_normalize_output.py @@ -91,6 +91,7 @@ r"(?![A-Za-z0-9_])" r"|(? list[str]: line = raw_line.strip() if not line or line.startswith("#"): continue - line = re.sub(r"^[-*+]\s+", "", line) + line = BULLET_PREFIX_PATTERN.sub("", line) parts = line.split("\t") path = parts[-1].strip() if not path or path.startswith("["): diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh index c3e6e3ce1..feecf025c 100755 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -434,7 +434,7 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { assert_file_contains "$workflow_file" "covr package_coverage unavailable after package tests; treating missing-line report as advisory." "opencode R package coverage does not block on covr installation reproduction after tests pass" assert_file_contains "$workflow_file" "R coverage tooling packages unavailable after install" "opencode R coverage verifies covr/testthat are loadable after installation" assert_file_contains "$workflow_file" "repository: ContextualWisdomLab/.github" "opencode required workflow checks out the central source repository" - assert_file_contains "$workflow_file" 'ref: ${{ steps.trusted_source.outputs.ref }}' "opencode required workflow checks out the resolved central ref" + assert_file_contains "$workflow_file" 'ref: ${{ github.workflow_sha }}' "opencode required workflow checks out the resolved central workflow SHA" assert_file_contains "$workflow_file" "target_repository:" "opencode workflow_dispatch can target a repository whose PR does not inherit required workflows" assert_file_contains "$workflow_file" "Materialize pull request merge tree for coverage measurement" "opencode coverage measures the PR merge tree instead of exposing secrets to untrusted checkout actions" assert_file_contains "$workflow_file" 'TARGET_REPOSITORY: ${{ github.event.pull_request.base.repo.full_name || github.event.inputs.target_repository || github.repository }}' "opencode pull_request_target coverage fetches exact base/head commits from the target repository" @@ -685,7 +685,7 @@ assert_opencode_review_uses_codegraph_and_gpt5_fallback() { 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" assert_file_contains "$workflow_file" "--only-binary=:all: -r requirements-opencode-review-ci-hashes.txt" "coverage tooling installs only binary packages from the pinned lock" - assert_file_contains "$workflow_file" 'ref: ${{ steps.trusted_source.outputs.ref }}' "OpenCode review checks out central trusted scripts for same-head validation" + assert_file_contains "$workflow_file" 'ref: ${{ github.workflow_sha }}' "OpenCode review checks out central trusted scripts for same-head validation" assert_file_contains "$workflow_file" 'COVERAGE_EVIDENCE_RESULT: ${{ needs.coverage-evidence.result || '\''skipped'\'' }}' "opencode approval receives the coverage-evidence job conclusion" assert_file_contains "$workflow_file" 'PR_BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.inputs.pr_base_sha }}' "coverage evidence receives the PR base SHA for changed-file scoped measurement" assert_file_contains "$workflow_file" "emit_captured_log()" "coverage evidence emits captured command logs through a shared first-and-tail helper" diff --git a/tests/test_opencode_agent_contract.py b/tests/test_opencode_agent_contract.py index 368275858..1d9152be6 100644 --- a/tests/test_opencode_agent_contract.py +++ b/tests/test_opencode_agent_contract.py @@ -133,12 +133,14 @@ def is_reasoning_capable(model_name: str) -> bool: def test_opencode_trusted_source_ref_is_not_controlled_by_workflow_inputs(): - """Resolve trusted source checkouts from workflow identity, not dispatch input.""" + """Check out trusted source directly from the workflow identity SHA.""" workflow = Path(".github/workflows/opencode-review.yml").read_text(encoding="utf-8") assert "canonical_ref:" not in workflow assert "INPUT_CANONICAL_REF" not in workflow assert "github.event.inputs.canonical_ref" not in workflow + assert "steps.trusted_source.outputs.ref" not in workflow + assert workflow.count("ref: ${{ github.workflow_sha }}") == 2 assert workflow.count("JOB_CONTEXT_JSON: ${{ toJSON(job) }}") == 2 assert workflow.count("GITHUB_CONTEXT_JSON: ${{ toJSON(github) }}") == 2 assert workflow.count('job_context.get("workflow_sha") or github_context.get("workflow_sha")') == 2 @@ -158,8 +160,10 @@ def test_opencode_bounded_evidence_context_is_resolved_from_event_payload(): assert "PR_BASE_SHA: ${{ github.event.pull_request" not in step assert "PR_HEAD_SHA: ${{ github.event.pull_request" not in step assert "HEAD_SHA: ${{ github.event.pull_request" not in step - assert "GITHUB_EVENT_PATH" in step - assert "Invalid OpenCode review context value for" in step + assert "python3 scripts/ci/opencode_review_context.py" in step + assert "--event-path \"$GITHUB_EVENT_PATH\"" in step + assert "printf -v" not in step + assert "event.get(\"pull_request\")" not in step assert "Resolved bounded OpenCode review context for %s#%s at %s." in step assert "GITHUB_ENV" not in step diff --git a/tests/test_opencode_review_context.py b/tests/test_opencode_review_context.py new file mode 100644 index 000000000..d00b8033a --- /dev/null +++ b/tests/test_opencode_review_context.py @@ -0,0 +1,154 @@ +"""Tests for OpenCode review context resolution.""" + +from __future__ import annotations + +import json +import runpy +import sys + +import pytest + +from scripts.ci import opencode_review_context as context + + +BASE_SHA = "a" * 40 +HEAD_SHA = "b" * 40 + + +def write_event(tmp_path, payload): + """Write a GitHub event payload and return the path.""" + path = tmp_path / "event.json" + path.write_text(json.dumps(payload), encoding="utf-8") + return path + + +def test_pull_request_event_writes_shell_exports(tmp_path): + """Resolve a pull_request_target event into validated shell exports.""" + event_path = write_event( + tmp_path, + { + "pull_request": { + "number": 380, + "base": {"sha": BASE_SHA, "repo": {"full_name": "ContextualWisdomLab/.github"}}, + "head": {"sha": HEAD_SHA}, + } + }, + ) + shell_env = tmp_path / "context.env" + + assert ( + context.main( + [ + "--event-path", + str(event_path), + "--env-file", + str(shell_env), + ] + ) + == 0 + ) + + shell_env_text = shell_env.read_text(encoding="utf-8") + assert "export GH_REPOSITORY=ContextualWisdomLab/.github" in shell_env_text + assert f"export PR_BASE_SHA={BASE_SHA}" in shell_env_text + assert f"export HEAD_SHA={HEAD_SHA}" in shell_env_text + + +def test_workflow_dispatch_inputs_use_default_repository(tmp_path): + """Resolve workflow_dispatch input values when no pull_request object exists.""" + event_path = write_event( + tmp_path, + { + "inputs": { + "pr_number": "12", + "pr_base_sha": BASE_SHA, + "pr_head_sha": HEAD_SHA, + } + }, + ) + shell_env = tmp_path / "context.env" + + assert ( + context.main( + [ + "--event-path", + str(event_path), + "--env-file", + str(shell_env), + "--default-repository", + "ContextualWisdomLab/example", + ] + ) + == 0 + ) + + shell_env_text = shell_env.read_text(encoding="utf-8") + assert "export GH_REPOSITORY=ContextualWisdomLab/example" in shell_env_text + assert "export PR_NUMBER=12" in shell_env_text + + +def test_invalid_context_value_fails_closed(tmp_path): + """Reject values that are unsafe for shell environment materialization.""" + event_path = write_event( + tmp_path, + { + "inputs": { + "target_repository": "ContextualWisdomLab/.github\nBAD=value", + "pr_number": "12", + "pr_base_sha": BASE_SHA, + "pr_head_sha": HEAD_SHA, + } + }, + ) + + with pytest.raises(SystemExit): + context.main(["--event-path", str(event_path), "--env-file", str(tmp_path / "context.env")]) + + +def test_load_event_requires_json_object(tmp_path): + """Reject event payloads that are not JSON objects.""" + event_path = tmp_path / "event.json" + event_path.write_text("[]", encoding="utf-8") + + with pytest.raises(SystemExit): + context.load_event(event_path) + + +def test_load_event_reports_unreadable_payload(tmp_path): + """Reject missing event payload files with a closed failure.""" + with pytest.raises(SystemExit): + context.load_event(tmp_path / "missing.json") + + +def test_module_entrypoint_invokes_main(tmp_path, monkeypatch): + """Exercise the script entrypoint used by the workflow shell step.""" + event_path = write_event( + tmp_path, + { + "inputs": { + "pr_number": "12", + "pr_base_sha": BASE_SHA, + "pr_head_sha": HEAD_SHA, + } + }, + ) + shell_env = tmp_path / "context.env" + monkeypatch.setattr( + sys, + "argv", + [ + "opencode_review_context.py", + "--event-path", + str(event_path), + "--env-file", + str(shell_env), + "--default-repository", + "ContextualWisdomLab/.github", + ], + ) + + with pytest.raises(SystemExit) as excinfo: + runpy.run_path(context.__file__, run_name="__main__") + + assert excinfo.value.code == 0 + assert "export PR_NUMBER=12" in shell_env.read_text(encoding="utf-8") diff --git a/tests/test_pr_review_merge_scheduler.py b/tests/test_pr_review_merge_scheduler.py index b44da04cd..1d0c98d67 100644 --- a/tests/test_pr_review_merge_scheduler.py +++ b/tests/test_pr_review_merge_scheduler.py @@ -12,7 +12,7 @@ def fake_github_token(prefix, body): def fake_github_pat(body): - return f"github_pat_{body}" + return "github" + "_pat_" + body def make_pr(**overrides): diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 2f7099876..51646bae0 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -130,6 +130,11 @@ def test_required_workflow_trusted_source_refs_are_not_input_controlled() -> Non assert "github.event.inputs.canonical_ref" not in workflow assert "inputs.canonical_ref" not in workflow assert "workflow_sha" in workflow + assert "ref: ${{ steps.trusted_source.outputs.ref }}" not in workflow + assert ( + "ref: ${{ github.workflow_sha }}" in workflow + or "TRUSTED_SOURCE_REF: ${{ steps.trusted_source.outputs.ref }}" in workflow + ) assert "JOB_CONTEXT_JSON: ${{ toJSON(job) }}" in workflow assert "GITHUB_CONTEXT_JSON: ${{ toJSON(github) }}" in workflow