diff --git a/.castiron.stats.yml b/.castiron.stats.yml index 90c75d40bf..26d99d11aa 100644 --- a/.castiron.stats.yml +++ b/.castiron.stats.yml @@ -1,8 +1,8 @@ schema_version: 1 -generation_id: 14743cd4-a53e-4fed-ba72-d5e23592153e -openapi_spec_hash: a99ded1ea34cf528a9cd5f064167f26a -openapi_transformed_spec_hash: e24c9d9339620c3cce8bbdd80e9ef8ed +generation_id: ddf51c0b-5978-487f-be21-e379112a927a +openapi_spec_hash: a85edbfc22ff719d064bce2705c7394e +openapi_transformed_spec_hash: f8e7644df5aee22dfcd0ea2b70942054 config_hash: 85382dd94c503b5d225adc7636a77c9f -codegen_sha: 66cd6dedd5d1b60732d891f911deb07347a8f068 -codegen_hash: 0e1cb892e3631438e55b55edd14899551f458be8d073e13d2c191730297562d6 -public_codegen_sha: 6356986f823c01fd602da9f64ef414c01db4619c +codegen_sha: 310aa46b5b69a9e4a3d0dd78fa47797d6cae2746 +codegen_hash: 5188f6aac875d009719a2f3702c2824068b7ce0780fe001f8a791735d8212a8b +public_codegen_sha: 0d3e70da47bb645fbfd0dd16fdd37ea10ee981f5 diff --git a/.github/workflows/castiron-custom-code-comment.yml b/.github/workflows/castiron-custom-code-comment.yml index 4a71d06c37..dabe1b3817 100644 --- a/.github/workflows/castiron-custom-code-comment.yml +++ b/.github/workflows/castiron-custom-code-comment.yml @@ -13,9 +13,63 @@ concurrency: cancel-in-progress: false jobs: + compute: + name: Compute trusted custom-code report + if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.path == '.github/workflows/castiron-custom-code.yml' + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + actions: read + pull-requests: read + outputs: + number: ${{ steps.report.outputs.number }} + artifact-id: ${{ steps.artifact.outputs.artifact-id }} + artifact-run-attempt: ${{ github.run_attempt }} + steps: + - name: Check out the trusted reporter + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + with: + ref: ${{ github.workflow_sha }} + persist-credentials: false + + - name: Compute from the current pull request Git objects + id: report + env: + GH_TOKEN: ${{ github.token }} + GIT_CONFIG_COUNT: '2' + GIT_CONFIG_KEY_0: credential.helper + GIT_CONFIG_VALUE_0: '' + GIT_CONFIG_KEY_1: credential.https://github.com.helper + GIT_CONFIG_VALUE_1: '!gh auth git-credential' + REPOSITORY: ${{ github.repository }} + RUN_ID: ${{ github.event.workflow_run.id }} + RUN_ATTEMPT: ${{ github.event.workflow_run.run_attempt }} + run: | + python3 -I scripts/castiron/custom_code_report.py trusted-report \ + --repo "$RUNNER_TEMP/castiron-objects.git" \ + --repository "$REPOSITORY" --run-id "$RUN_ID" --run-attempt "$RUN_ATTEMPT" \ + --out "$RUNNER_TEMP/castiron-custom-code" + if test -f "$RUNNER_TEMP/castiron-custom-code/context.json"; then + number=$(jq -er '.pr' "$RUNNER_TEMP/castiron-custom-code/context.json") + printf 'number=%s\n' "$number" >> "$GITHUB_OUTPUT" + cat "$RUNNER_TEMP/castiron-custom-code/summary.md" >> "$GITHUB_STEP_SUMMARY" + fi + + - name: Upload the trusted report and patch + id: artifact + if: steps.report.outputs.number != '' + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: castiron-custom-code-${{ github.run_id }}-${{ github.run_attempt }} + path: ${{ runner.temp }}/castiron-custom-code/ + if-no-files-found: error + retention-days: 7 + comment: name: Update custom-code comment - if: github.event.workflow_run.event == 'pull_request' && github.event.workflow_run.path == '.github/workflows/castiron-custom-code.yml' + needs: compute + if: always() && !cancelled() && (needs.compute.result == 'failure' || needs.compute.outputs.number != '') runs-on: ubuntu-latest timeout-minutes: 5 permissions: @@ -29,40 +83,31 @@ jobs: ref: ${{ github.workflow_sha }} persist-credentials: false - - name: Download the completed run's report + - name: Download this workflow's trusted report + if: needs.compute.result == 'success' uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: - github-token: ${{ github.token }} - run-id: ${{ github.event.workflow_run.id }} - name: castiron-custom-code-${{ github.event.workflow_run.id }}-${{ github.event.workflow_run.run_attempt }} + artifact-ids: ${{ needs.compute.outputs.artifact-id }} + merge-multiple: true path: ${{ runner.temp }}/castiron-custom-code - - name: Validate report context - id: context - env: - REPOSITORY: ${{ github.repository }} - RUN_ID: ${{ github.event.workflow_run.id }} - RUN_ATTEMPT: ${{ github.event.workflow_run.run_attempt }} - run: | - number=$(jq -er --arg repository "$REPOSITORY" --argjson run "$RUN_ID" \ - --argjson attempt "$RUN_ATTEMPT" \ - 'select(.repository == $repository and .run == $run and .attempt == $attempt) | .pr | select(type == "number" and . > 0 and . == floor)' \ - "$RUNNER_TEMP/castiron-custom-code/context.json") - printf 'number=%s\n' "$number" >> "$GITHUB_OUTPUT" - - name: Create or update the single report comment id: publish + if: needs.compute.result == 'success' env: GH_TOKEN: ${{ github.token }} REPOSITORY: ${{ github.repository }} - PR_NUMBER: ${{ steps.context.outputs.number }} + PR_NUMBER: ${{ needs.compute.outputs.number }} RUN_ID: ${{ github.event.workflow_run.id }} RUN_ATTEMPT: ${{ github.event.workflow_run.run_attempt }} + ARTIFACT_RUN_ID: ${{ github.run_id }} + ARTIFACT_RUN_ATTEMPT: ${{ needs.compute.outputs.artifact-run-attempt }} run: | python3 -I scripts/castiron/custom_code_report.py comment \ --report "$RUNNER_TEMP/castiron-custom-code/report.json" \ --repository "$REPOSITORY" --pr "$PR_NUMBER" --run-id "$RUN_ID" \ - --run-attempt "$RUN_ATTEMPT" + --run-attempt "$RUN_ATTEMPT" \ + --artifact-run-id "$ARTIFACT_RUN_ID" --artifact-run-attempt "$ARTIFACT_RUN_ATTEMPT" - name: Publish a trusted failure status if: always() && !cancelled() && steps.publish.outcome != 'success' diff --git a/.github/workflows/castiron-custom-code.yml b/.github/workflows/castiron-custom-code.yml index c78811d7ed..ef23bfc8b1 100644 --- a/.github/workflows/castiron-custom-code.yml +++ b/.github/workflows/castiron-custom-code.yml @@ -13,7 +13,7 @@ concurrency: cancel-in-progress: false env: - REPORTER_SHA256: 73ecd6290e9803b0d0a93af4ca4dccdbf8648cbd5c0cce51ea65fce28c7da79f + REPORTER_SHA256: ac48ca88e9f7ad57195038157e99f055c0cd3dac8de856e4d102dca807766d4a jobs: report: diff --git a/api_reference/openapi.transformed.yml b/api_reference/openapi.transformed.yml index 8cda8f8cb0..f14d867a01 100644 --- a/api_reference/openapi.transformed.yml +++ b/api_reference/openapi.transformed.yml @@ -35398,6 +35398,8 @@ components: - type: string enum: - gpt-image-1.5 + - gpt-image-2 + - gpt-image-2-2026-04-21 - gpt-image-1 - gpt-image-1-mini - chatgpt-image-latest @@ -35405,7 +35407,7 @@ components: x-oaiTypeLabel: string default: gpt-image-1.5 example: gpt-image-1.5 - description: The model to use for image editing. + description: The GPT image model to use for image editing, including `gpt-image-2` and its dated snapshot `gpt-image-2-2026-04-21`. images: type: array minItems: 1 @@ -39136,7 +39138,10 @@ components: - gpt-image-1.5 - chatgpt-image-latest description: | - The image generation model to use. Default: `gpt-image-1`. + The image generation model to use. One of `gpt-image-1`, + `gpt-image-1-mini`, `gpt-image-1.5`, `gpt-image-2`, + `gpt-image-2-2026-04-21`, or `chatgpt-image-latest`. Default: + `gpt-image-1`. default: gpt-image-1 quality: type: string @@ -68512,7 +68517,10 @@ components: - gpt-image-1.5 - chatgpt-image-latest description: | - The image generation model to use. Default: `gpt-image-1`. + The image generation model to use. One of `gpt-image-1`, + `gpt-image-1-mini`, `gpt-image-1.5`, `gpt-image-2`, + `gpt-image-2-2026-04-21`, or `chatgpt-image-latest`. Default: + `gpt-image-1`. default: gpt-image-1 quality: type: string diff --git a/scripts/castiron/README.md b/scripts/castiron/README.md index 16dc15c266..70e2412382 100644 --- a/scripts/castiron/README.md +++ b/scripts/castiron/README.md @@ -16,8 +16,12 @@ Its hash format is documented in the reporter. Only `.github/actions/` and `.github/workflows/` are excluded from the content hash. The read-only pull-request workflow runs on every branch, including drafts and -forks. A separate `workflow_run` publisher reads its report as untrusted data and -uses only code from the trusted default branch to update the PR comment. The +forks. A separate read-only `workflow_run` job computes the authoritative report from +current, GitHub-associated base/head Git objects using the trusted workflow +revision. It fetches those objects into a new bare repository and never checks +out or executes PR code. The comment-writing job consumes only the artifact +from that trusted job, rechecks freshness, and links to its report and patch. +PR-produced reports are advisory run output, not the published assessment. The publisher becomes active once its workflow is on the default branch. No branch allowlist or repository variable is needed. Never execute PR-controlled code with write credentials. Changing either workflow may require one-time AM permission. diff --git a/scripts/castiron/custom_code_report.py b/scripts/castiron/custom_code_report.py index 1083c1be15..3e6ed19e89 100644 --- a/scripts/castiron/custom_code_report.py +++ b/scripts/castiron/custom_code_report.py @@ -767,7 +767,14 @@ def api(method: str, path: str, payload: dict[str, Any] | None = None) -> Any: def publish_comment( - report: dict[str, Any], repository: str, number: int, run_id: int, run_attempt: int + report: dict[str, Any], + repository: str, + number: int, + run_id: int, + run_attempt: int, + *, + artifact_run_id: int = 0, + artifact_run_attempt: int = 0, ) -> str: if not REPOSITORY.fullmatch(repository) or min(number, run_id, run_attempt) <= 0: raise ReportError("invalid GitHub publication target") @@ -794,12 +801,14 @@ def publish_comment( raise ReportError("workflow run does not match report PR/head") if run["run_attempt"] != run_attempt: return "Skipped stale report" + artifact_run_id = artifact_run_id or run_id + artifact_run_attempt = artifact_run_attempt or run_attempt body = render_report( report, - f"https://github.com/{repository}/actions/runs/{run_id}", + f"https://github.com/{repository}/actions/runs/{artifact_run_id}", repository=repository, - run_id=run_id, - run_attempt=run_attempt, + run_id=artifact_run_id, + run_attempt=artifact_run_attempt, ) body += f"\n\n" found = None @@ -834,6 +843,88 @@ def publish_comment( return str(result["html_url"]) +def write_report( + repo: Path, + base: str, + head: str, + out: Path, + *, + fetch: bool, + require_head_hash: bool, + public: bool, +) -> dict[str, Any]: + out.mkdir(parents=True, exist_ok=True) + try: + report, patch = build_report( + repo, base, head, fetch=fetch, require_head_hash=require_head_hash, public=public + ) + except (ReportError, UnicodeError, KeyError, ValueError) as exc: + report = { + "schema_version": 1, + "status": "error", + "target_base_sha": require_sha(base), + "head_sha": require_sha(head), + "error": str(exc), + } + patch = b"" + (out / "report.json").write_text(json.dumps(report, indent=2, sort_keys=True) + "\n") + (out / "custom-code.patch").write_bytes(patch) + summary = render_report(report) + (out / "summary.md").write_text(summary) + sys.stdout.write(summary) + return report + + +def trusted_report( + repo: Path, repository: str, run_id: int, run_attempt: int, out: Path +) -> None: + """Recompute from GitHub-associated Git objects, never from PR-produced artifacts.""" + if not REPOSITORY.fullmatch(repository) or min(run_id, run_attempt) <= 0: + raise ReportError("invalid GitHub report target") + root = f"repos/{repository}" + run = api("GET", f"{root}/actions/runs/{run_id}") + if ( + run["event"] != "pull_request" + or run.get("path", "").split("@", 1)[0] != ".github/workflows/castiron-custom-code.yml" + or run["status"] != "completed" + ): + raise ReportError("unexpected source workflow run") + if run["run_attempt"] != run_attempt: + return + head = require_sha(run["head_sha"]) + associated = run["pull_requests"] or api("GET", f"{root}/commits/{head}/pulls?per_page=100") + current: list[tuple[int, str]] = [] + for number in sorted({int(pr["number"]) for pr in associated}): + if number <= 0: + raise ReportError("invalid associated pull request") + pull = api("GET", f"{root}/pulls/{number}") + if ( + pull["state"] == "open" + and pull["head"]["sha"] == head + and pull["base"]["repo"]["full_name"] == repository + ): + current.append((number, require_sha(pull["base"]["sha"]))) + if not current: + return + if len(current) != 1: + raise ReportError("workflow run has multiple current pull requests") + number, base = current[0] + public = not api("GET", root)["private"] + # This must be a new, bare repository: no PR worktree, hooks, configuration, + # submodules, or Python imports can affect the trusted reporter. + repo.mkdir() + git(repo, "init", "--quiet", "--bare") + git(repo, "remote", "add", "origin", f"https://github.com/{repository}.git") + git(repo, "fetch", "--quiet", "--no-tags", "origin", base, head) + write_report(repo, base, head, out, fetch=True, require_head_hash=True, public=public) + (out / "context.json").write_text( + json.dumps( + {"pr": number, "repository": repository, "run": run_id, "attempt": run_attempt} + ) + + "\n" + ) + + def main() -> int: parser = argparse.ArgumentParser(description=__doc__) commands = parser.add_subparsers(dest="command", required=True) @@ -848,6 +939,12 @@ def main() -> int: reporting.add_argument("--fetch", action="store_true") reporting.add_argument("--require-head-hash", action="store_true") reporting.add_argument("--public", action="store_true") + trusted = commands.add_parser("trusted-report") + trusted.add_argument("--repo", type=Path, required=True) + trusted.add_argument("--repository", required=True) + trusted.add_argument("--run-id", type=int, required=True) + trusted.add_argument("--run-attempt", type=int, required=True) + trusted.add_argument("--out", type=Path, required=True) preparing = commands.add_parser("prepare-public") preparing.add_argument("--source-repo", type=Path, required=True) preparing.add_argument("--source-base", required=True) @@ -861,6 +958,8 @@ def main() -> int: commenting.add_argument("--pr", type=int, required=True) commenting.add_argument("--run-id", type=int, required=True) commenting.add_argument("--run-attempt", type=int, required=True) + commenting.add_argument("--artifact-run-id", type=int, default=0) + commenting.add_argument("--artifact-run-attempt", type=int, default=0) args = parser.parse_args() try: if args.command == "hash": @@ -879,41 +978,34 @@ def main() -> int: ) + "\n" ) + elif args.command == "trusted-report": + trusted_report(args.repo, args.repository, args.run_id, args.run_attempt, args.out) elif args.command == "comment": if args.report.stat().st_size > 5_000_000: raise ReportError("report artifact is too large") report = json.loads(args.report.read_text()) sys.stdout.write( - publish_comment(report, args.repository, args.pr, args.run_id, args.run_attempt) + publish_comment( + report, + args.repository, + args.pr, + args.run_id, + args.run_attempt, + artifact_run_id=args.artifact_run_id, + artifact_run_attempt=args.artifact_run_attempt, + ) + "\n" ) else: - args.out.mkdir(parents=True, exist_ok=True) - try: - report, patch = build_report( - args.repo, - args.base, - args.head, - fetch=args.fetch, - require_head_hash=args.require_head_hash, - public=args.public, - ) - except (ReportError, UnicodeError, KeyError, ValueError) as exc: - report = { - "schema_version": 1, - "status": "error", - "target_base_sha": require_sha(args.base), - "head_sha": require_sha(args.head), - "error": str(exc), - } - patch = b"" - (args.out / "report.json").write_text( - json.dumps(report, indent=2, sort_keys=True) + "\n" + report = write_report( + args.repo, + args.base, + args.head, + args.out, + fetch=args.fetch, + require_head_hash=args.require_head_hash, + public=args.public, ) - (args.out / "custom-code.patch").write_bytes(patch) - summary = render_report(report) - (args.out / "summary.md").write_text(summary) - sys.stdout.write(summary) return 0 if report["status"] == "ok" else 1 except (ReportError, OSError, subprocess.TimeoutExpired) as exc: sys.stderr.write(f"Castiron custom-code report: {exc}\n") diff --git a/scripts/castiron/test_custom_code_report.py b/scripts/castiron/test_custom_code_report.py index 6e9d938f00..cc3663034f 100644 --- a/scripts/castiron/test_custom_code_report.py +++ b/scripts/castiron/test_custom_code_report.py @@ -12,6 +12,7 @@ import textwrap import unittest from pathlib import Path +from typing import Any from unittest import mock import custom_code_report as report @@ -328,6 +329,260 @@ def test_workflow_reports_all_branches_without_write_credentials(self) -> None: self.assertNotIn("ref: ${{ github.event.pull_request.head.sha }}", publisher) self.assertIn("persist-credentials: false", publisher) self.assertIn("--report", publisher) + compute, comment = publisher.split("\n comment:\n", 1) + self.assertNotIn("pull-requests: write", compute) + self.assertIn("pull-requests: read", compute) + self.assertIn(" trusted-report ", compute) + self.assertNotIn("download-artifact@", compute) + self.assertNotIn("unittest", compute) + self.assertIn("needs: compute", comment) + self.assertIn("artifact-ids: ${{ needs.compute.outputs.artifact-id }}", comment) + self.assertNotIn("run-id: ${{ github.event.workflow_run.id }}", comment) + self.assertNotIn("git fetch", comment) + self.assertIn("--artifact-run-id", comment) + digest = hashlib.sha256( + (workflows.parents[1] / "scripts/castiron/custom_code_report.py").read_bytes() + ).hexdigest() + self.assertIn(f"REPORTER_SHA256: {digest}", producer) + + def test_comment_only_rerun_links_to_the_compute_artifact_attempt(self) -> None: + workflow = ( + Path(__file__).resolve().parents[2] + / ".github/workflows/castiron-custom-code-comment.yml" + ).read_text() + compute, comment = workflow.split("\n comment:\n", 1) + + def field(section: str, prefix: str) -> str: + return next( + line.removeprefix(prefix) + for line in section.splitlines() + if line.startswith(prefix) + ) + + def resolve(value: str, context: dict[str, str]) -> str: + for key, replacement in context.items(): + value = value.replace("${{ " + key + " }}", replacement) + self.assertNotIn("${{", value) + return value + + # A successful compute job's outputs survive a comment-only rerun. + compute_context = {"github.run_id": "9", "github.run_attempt": "3"} + uploaded_name = resolve(field(compute, " name: "), compute_context) + saved_attempt = resolve(field(compute, " artifact-run-attempt: "), compute_context) + comment_context = { + "github.run_id": "9", + "github.run_attempt": "4", + "needs.compute.outputs.artifact-run-attempt": saved_attempt, + } + artifact_attempt = resolve( + field(comment, " ARTIFACT_RUN_ATTEMPT: "), comment_context + ) + self.assertEqual(uploaded_name, "castiron-custom-code-9-3") + self.assertEqual(artifact_attempt, "3") + + _, base = self.baseline() + result, _ = report.build_report(self.repo, base, base) + pull = {"state": "open", "head": {"sha": base}, "base": {"sha": base}} + run = { + "event": "pull_request", + "path": ".github/workflows/castiron-custom-code.yml", + "head_sha": base, + "run_attempt": 1, + "pull_requests": [{"number": 1}], + } + with mock.patch.object( + report, "api", side_effect=[pull, run, [], pull, {"html_url": "published"}] + ) as api: + self.assertEqual( + report.publish_comment( + result, + "openai/example", + 1, + 2, + 1, + artifact_run_id=9, + artifact_run_attempt=int(artifact_attempt), + ), + "published", + ) + body = api.call_args.args[2]["body"] + self.assertIn(f"--name {uploaded_name}", body) + self.assertNotIn("--name castiron-custom-code-9-4", body) + self.assertIn("castiron:run:v1:2:1", body) + + def test_trusted_report_recomputes_pr_output_in_a_bare_repository(self) -> None: + generated, _ = self.baseline() + content_hash = report.hash_codegen_commit(self.repo, generated) + snapshot = report.create_public_snapshot( + self.repo, + self.git("rev-parse", f"{generated}^{{tree}}"), + GENERATION, + content_hash, + "codegen/public-test", + None, + ) + self.git("branch", "codegen/public-test", snapshot) + stats = (self.repo / ".castiron.stats.yml").read_text() + self.write(".castiron.stats.yml", stats + f"public_codegen_sha: {snapshot}\n") + base = self.commit() + legitimate, _ = report.build_report(self.repo, base, base, require_head_hash=True) + self.write("generated.py", "generated\n# custom\n") + # Neither a replacement reporter nor its claimed result may be executed + # or read by the trusted job. + self.write( + "scripts/castiron/custom_code_report.py", "raise RuntimeError('PR code ran')\n" + ) + self.write("report.json", json.dumps(legitimate)) + head = self.commit() + broken_stats = ( + (self.repo / ".castiron.stats.yml").read_text().replace(content_hash, "0" * 64) + ) + self.write(".castiron.stats.yml", broken_stats) + broken = self.commit() + remote = self.repo / "public.git" + self.git("clone", "--bare", str(self.repo), str(remote)) + real_git = report.git + + def local_git(repo: Path, *args: str, input_bytes: bytes | None = None) -> bytes: + if args[:3] == ("remote", "add", "origin"): + self.assertEqual(args[3], "https://github.com/openai/example.git") + args = (*args[:3], str(remote)) + self.assertNotIn("checkout", args) + return real_git(repo, *args, input_bytes=input_bytes) + + for label, revision in (("genuine", base), ("custom", head), ("broken", broken)): + with self.subTest(label=label): + calls: list[tuple[str, str]] = [] + bodies: list[str] = [] + pull = { + "state": "open", + "head": {"sha": revision}, + "base": {"sha": base, "repo": {"full_name": "openai/example"}}, + } + run: dict[str, Any] = { + "event": "pull_request", + "status": "completed", + "path": ".github/workflows/castiron-custom-code.yml", + "head_sha": revision, + "run_attempt": 1, + "pull_requests": [], + } + forged: dict[str, Any] = {**legitimate, "head_sha": revision, "files": []} + self.assertIn("Generated baselines verified", report.render_report(forged)) + producer = self.repo / f"producer-{label}" + producer.mkdir() + (producer / "report.json").write_text(json.dumps(forged)) + + def fake_api( + method: str, path: str, payload: dict[str, Any] | None = None + ) -> Any: + calls.append((method, path)) + if method == "GET": + responses: dict[str, Any] = { + "repos/openai/example": {"private": False}, + "repos/openai/example/actions/runs/2": run, + f"repos/openai/example/commits/{revision}/pulls?per_page=100": [ + {"number": 1} + ], + "repos/openai/example/pulls/1": pull, + "repos/openai/example/issues/1/comments?per_page=100&page=1": [], + } + if path in responses: + return responses[path] + if ( + method == "POST" + and path == "repos/openai/example/issues/1/comments" + and payload + ): + bodies.append(payload["body"]) + return { + "html_url": "https://github.com/openai/example/pull/1#issuecomment-1" + } + raise AssertionError(f"unexpected API call: {method} {path}") + + objects = self.repo / f"objects-{label}.git" + out = self.repo / f"trusted-{label}" + with ( + mock.patch.object(report, "api", side_effect=fake_api), + mock.patch.object(report, "git", side_effect=local_git), + ): + report.trusted_report(objects, "openai/example", 2, 1, out) + self.assertTrue(all(method == "GET" for method, _ in calls)) + self.assertEqual( + real_git(objects, "rev-parse", "--is-bare-repository"), b"true\n" + ) + self.assertFalse((objects / "scripts").exists()) + actual = json.loads((out / "report.json").read_text()) + report.publish_comment( + actual, + "openai/example", + 1, + 2, + 1, + artifact_run_id=9, + artifact_run_attempt=3, + ) + self.assertEqual(len(bodies), 1) + body = bodies[0] + self.assertIn("castiron:run:v1:2:1", body) + self.assertIn("/actions/runs/9", body) + if label == "broken": + self.assertIn("Report unavailable", body) + self.assertNotIn("Generated baselines verified", body) + elif label == "custom": + self.assertIn("1 newly customized", body) + self.assertIn("generated.py", body) + self.assertIn(b"+# custom", (out / "custom-code.patch").read_bytes()) + self.assertNotIn("No new custom-code files detected", body) + else: + self.assertIn("No new custom-code files detected", body) + self.assertIn("Generated baselines verified", body) + self.assertIn("--name castiron-custom-code-9-3", body) + + def test_trusted_report_rejects_invalid_or_stale_association_before_fetch(self) -> None: + run = { + "event": "pull_request", + "status": "completed", + "path": ".github/workflows/castiron-custom-code.yml", + "head_sha": "a" * 40, + "run_attempt": 1, + "pull_requests": [{"number": 1}], + } + pull = { + "state": "open", + "head": {"sha": "a" * 40}, + "base": {"sha": "b" * 40, "repo": {"full_name": "openai/example"}}, + } + cases: list[tuple[list[Any], bool]] = [ + ([{**run, "path": "other.yml"}], True), + ([{**run, "status": "in_progress"}], True), + ([{**run, "run_attempt": 2}], False), + ([run, {**pull, "state": "closed"}], False), + ([run, {**pull, "head": {"sha": "c" * 40}}], False), + ( + [run, {**pull, "base": {"sha": "b" * 40, "repo": {"full_name": "other/repo"}}}], + False, + ), + ([{**run, "pull_requests": []}, []], False), + ([{**run, "pull_requests": [{"number": 1}, {"number": 2}]}, pull, pull], True), + ] + for responses, raises in cases: + with ( + self.subTest(responses=responses), + mock.patch.object(report, "api", side_effect=responses), + mock.patch.object(report, "git") as git, + ): + if raises: + with self.assertRaises(report.ReportError): + report.trusted_report( + self.repo / "objects", "openai/example", 2, 1, self.repo / "out" + ) + else: + report.trusted_report( + self.repo / "objects", "openai/example", 2, 1, self.repo / "out" + ) + git.assert_not_called() + self.assertFalse((self.repo / "out").exists()) def test_removals_include_changed_baselines_but_not_handwritten_only_files(self) -> None: _, base = self.baseline() diff --git a/scripts/utils/validate-httpx2-wheel.py b/scripts/utils/validate-httpx2-wheel.py index 42bf72d177..2d3e0bba5d 100644 --- a/scripts/utils/validate-httpx2-wheel.py +++ b/scripts/utils/validate-httpx2-wheel.py @@ -13,6 +13,7 @@ BASE_TEST = ROOT / "tests/test_httpx2_base.py" HTTPX2_TEST = ROOT / "tests/test_httpx2.py" LEGACY_TEST = ROOT / "tests/test_httpx_compat.py" +TLS_TEST = ROOT / "tests/test_tls_hostname.py" def venv_python(environment_path: Path) -> Path: @@ -109,8 +110,8 @@ def main() -> None: run_case( wheel, extra=None, - tests=[LEGACY_TEST], - dependencies=[*common, "httpx-aiohttp>=0.2.0,<0.3"], + tests=[LEGACY_TEST, TLS_TEST], + dependencies=[*common, "httpx-aiohttp>=0.2.0,<0.3", "cryptography>=50.0.0"], legacy=True, ) print("Validated HTTPX2-only base and aiohttp installs plus isolated legacy HTTPX/aiohttp compatibility") diff --git a/src/openai/_base_client.py b/src/openai/_base_client.py index 23813a2bb0..f195d04816 100644 --- a/src/openai/_base_client.py +++ b/src/openai/_base_client.py @@ -576,9 +576,6 @@ def _build_request( if params and prepared_url.query: params = {**dict(prepared_url.params.items()), **params} prepared_url = prepared_url.copy_with(raw_path=prepared_url.raw_path.split(b"?", 1)[0]) - if "_" in prepared_url.host: - # work around https://github.com/encode/httpx/discussions/2880 - kwargs["extensions"] = {"sni_hostname": prepared_url.host.replace("_", "-")} is_body_allowed = options.method.lower() != "get" diff --git a/src/openai/lib/_azure_websocket.py b/src/openai/lib/_azure_websocket.py index 36620cd21f..fb47d6a09e 100644 --- a/src/openai/lib/_azure_websocket.py +++ b/src/openai/lib/_azure_websocket.py @@ -1,23 +1,11 @@ from __future__ import annotations -from typing_extensions import override - -from websockets.uri import parse_uri -from websockets.exceptions import SecurityError -from websockets.asyncio.client import connect +from ._websocket import _WebSocketConnect __all__ = ["_AzureWebSocketConnect"] -class _AzureWebSocketConnect(connect): +class _AzureWebSocketConnect(_WebSocketConnect): """Keep Azure's WebSocket authentication on the original origin.""" - @override - def process_redirect(self, exc: Exception) -> Exception | str: - uri_or_exc = super().process_redirect(exc) - if isinstance(uri_or_exc, str): - current = parse_uri(self.uri) - target = parse_uri(uri_or_exc) - if (current.secure, current.host, current.port) != (target.secure, target.host, target.port): - return SecurityError("Cross-origin Azure WebSocket redirects are not allowed") - return uri_or_exc + _redirect_error_message = "Cross-origin Azure WebSocket redirects are not allowed" diff --git a/src/openai/lib/_websocket.py b/src/openai/lib/_websocket.py new file mode 100644 index 0000000000..5ca93864df --- /dev/null +++ b/src/openai/lib/_websocket.py @@ -0,0 +1,25 @@ +from __future__ import annotations + +from typing_extensions import override + +from websockets.uri import parse_uri +from websockets.exceptions import SecurityError +from websockets.asyncio.client import connect + +__all__ = ["_WebSocketConnect"] + + +class _WebSocketConnect(connect): + """Keep WebSocket authentication on the original origin.""" + + _redirect_error_message = "Cross-origin WebSocket redirects are not allowed" + + @override + def process_redirect(self, exc: Exception) -> Exception | str: + uri_or_exc = super().process_redirect(exc) + if isinstance(uri_or_exc, str): + current = parse_uri(self.uri) + target = parse_uri(uri_or_exc) + if (current.secure, current.host, current.port) != (target.secure, target.host, target.port): + return SecurityError(self._redirect_error_message) + return uri_or_exc diff --git a/src/openai/lib/azure.py b/src/openai/lib/azure.py index aeae62168a..c7e61767a2 100644 --- a/src/openai/lib/azure.py +++ b/src/openai/lib/azure.py @@ -443,11 +443,12 @@ def _get_azure_ad_token(self) -> str | None: provider = self._azure_ad_token_provider if provider is not None: - token = provider() - if not token or not isinstance(token, str): # pyright: ignore[reportUnnecessaryIsInstance] - raise ValueError( - f"Expected `azure_ad_token_provider` argument to return a string but it returned {token}", - ) + token = cast(object, provider()) + if isinstance(token, str): + # Bypass subclass methods before validating or interpolating credentials. + token = str.__str__(token) + if not isinstance(token, str) or not token: + raise ValueError("Expected `azure_ad_token_provider` argument to return a non-empty string.") return token return None @@ -794,14 +795,15 @@ async def _get_azure_ad_token(self) -> str | None: provider = self._azure_ad_token_provider if provider is not None: - token = provider() + token = cast(object, provider()) if inspect.isawaitable(token): token = await token - if not token or not isinstance(cast(Any, token), str): - raise ValueError( - f"Expected `azure_ad_token_provider` argument to return a string but it returned {token}", - ) - return str(token) + if isinstance(token, str): + # Bypass subclass methods before validating or interpolating credentials. + token = str.__str__(token) + if not isinstance(token, str) or not token: + raise ValueError("Expected `azure_ad_token_provider` argument to return a non-empty string.") + return token return None diff --git a/src/openai/resources/beta/realtime/realtime.py b/src/openai/resources/beta/realtime/realtime.py index 5365001707..7124287c0e 100644 --- a/src/openai/resources/beta/realtime/realtime.py +++ b/src/openai/resources/beta/realtime/realtime.py @@ -352,7 +352,7 @@ async def __aenter__(self) -> AsyncRealtimeConnection: ``` """ try: - from websockets.asyncio.client import connect + from ....lib._websocket import _WebSocketConnect as connect except ImportError as exc: raise OpenAIError("You need to install `openai[realtime]` to use this method") from exc diff --git a/src/openai/resources/beta/responses/responses.py b/src/openai/resources/beta/responses/responses.py index 48025c7dea..c9d1120fcc 100644 --- a/src/openai/resources/beta/responses/responses.py +++ b/src/openai/resources/beta/responses/responses.py @@ -4514,7 +4514,7 @@ async def __aenter__(self) -> AsyncResponsesConnection: async def _connect_ws(self, extra_query: Query, extra_headers: Headers) -> AsyncWebSocketConnection: try: - from websockets.asyncio.client import connect + from ....lib._websocket import _WebSocketConnect as connect except ImportError as exc: raise OpenAIError("You need to install `openai[realtime]` to use this method") from exc diff --git a/src/openai/resources/beta/threads/messages.py b/src/openai/resources/beta/threads/messages.py index 66f57f9488..28cd019753 100644 --- a/src/openai/resources/beta/threads/messages.py +++ b/src/openai/resources/beta/threads/messages.py @@ -15,10 +15,7 @@ from ...._resource import SyncAPIResource, AsyncAPIResource from ...._response import to_streamed_response_wrapper, async_to_streamed_response_wrapper from ....pagination import SyncCursorPage, AsyncCursorPage -from ...._base_client import ( - AsyncPaginator, - make_request_options, -) +from ...._base_client import AsyncPaginator, make_request_options from ....types.beta.threads import message_list_params, message_create_params, message_update_params from ....types.beta.threads.message import Message from ....types.shared_params.metadata import Metadata diff --git a/src/openai/resources/completions.py b/src/openai/resources/completions.py index 160732d9a0..d43c3854b2 100644 --- a/src/openai/resources/completions.py +++ b/src/openai/resources/completions.py @@ -15,9 +15,7 @@ from .._resource import SyncAPIResource, AsyncAPIResource from .._response import to_streamed_response_wrapper, async_to_streamed_response_wrapper from .._streaming import Stream, AsyncStream -from .._base_client import ( - make_request_options, -) +from .._base_client import make_request_options from ..types.completion import Completion from ..types.chat.chat_completion_stream_options_param import ChatCompletionStreamOptionsParam diff --git a/src/openai/resources/fine_tuning/jobs/checkpoints.py b/src/openai/resources/fine_tuning/jobs/checkpoints.py index 2476b66014..1768305baf 100644 --- a/src/openai/resources/fine_tuning/jobs/checkpoints.py +++ b/src/openai/resources/fine_tuning/jobs/checkpoints.py @@ -11,10 +11,7 @@ from ...._resource import SyncAPIResource, AsyncAPIResource from ...._response import to_streamed_response_wrapper, async_to_streamed_response_wrapper from ....pagination import SyncCursorPage, AsyncCursorPage -from ...._base_client import ( - AsyncPaginator, - make_request_options, -) +from ...._base_client import AsyncPaginator, make_request_options from ....types.fine_tuning.jobs import checkpoint_list_params from ....types.fine_tuning.jobs.fine_tuning_job_checkpoint import FineTuningJobCheckpoint diff --git a/src/openai/resources/fine_tuning/jobs/jobs.py b/src/openai/resources/fine_tuning/jobs/jobs.py index a6046cdd0f..b5fdd0b0de 100644 --- a/src/openai/resources/fine_tuning/jobs/jobs.py +++ b/src/openai/resources/fine_tuning/jobs/jobs.py @@ -22,10 +22,7 @@ from ...._resource import SyncAPIResource, AsyncAPIResource from ...._response import to_streamed_response_wrapper, async_to_streamed_response_wrapper from ....pagination import SyncCursorPage, AsyncCursorPage -from ...._base_client import ( - AsyncPaginator, - make_request_options, -) +from ...._base_client import AsyncPaginator, make_request_options from ....types.fine_tuning import job_list_params, job_create_params, job_list_events_params from ....types.shared_params.metadata import Metadata from ....types.fine_tuning.fine_tuning_job import FineTuningJob diff --git a/src/openai/resources/models.py b/src/openai/resources/models.py index 41152fb91f..209505f1c7 100644 --- a/src/openai/resources/models.py +++ b/src/openai/resources/models.py @@ -12,10 +12,7 @@ from .._response import to_streamed_response_wrapper, async_to_streamed_response_wrapper from ..pagination import SyncPage, AsyncPage from ..types.model import Model -from .._base_client import ( - AsyncPaginator, - make_request_options, -) +from .._base_client import AsyncPaginator, make_request_options from ..types.model_deleted import ModelDeleted __all__ = ["Models", "AsyncModels"] diff --git a/src/openai/resources/realtime/realtime.py b/src/openai/resources/realtime/realtime.py index 62ba3e4e54..3cb815bddf 100644 --- a/src/openai/resources/realtime/realtime.py +++ b/src/openai/resources/realtime/realtime.py @@ -682,7 +682,7 @@ async def __aenter__(self) -> AsyncRealtimeConnection: async def _connect_ws(self, extra_query: Query, extra_headers: Headers) -> AsyncWebSocketConnection: try: - from websockets.asyncio.client import connect + from ...lib._websocket import _WebSocketConnect as connect except ImportError as exc: raise OpenAIError("You need to install `openai[realtime]` to use this method") from exc diff --git a/src/openai/resources/responses/responses.py b/src/openai/resources/responses/responses.py index 3f01319a38..2232b1a05d 100644 --- a/src/openai/resources/responses/responses.py +++ b/src/openai/resources/responses/responses.py @@ -4409,7 +4409,7 @@ async def __aenter__(self) -> AsyncResponsesConnection: async def _connect_ws(self, extra_query: Query, extra_headers: Headers) -> AsyncWebSocketConnection: try: - from websockets.asyncio.client import connect + from ...lib._websocket import _WebSocketConnect as connect except ImportError as exc: raise OpenAIError("You need to install `openai[realtime]` to use this method") from exc diff --git a/src/openai/types/beta/beta_tool.py b/src/openai/types/beta/beta_tool.py index 9e1e64bbf2..57c659c967 100644 --- a/src/openai/types/beta/beta_tool.py +++ b/src/openai/types/beta/beta_tool.py @@ -302,7 +302,11 @@ class ImageGeneration(BaseModel): ], None, ] = None - """The image generation model to use. Default: `gpt-image-1`.""" + """The image generation model to use. + + One of `gpt-image-1`, `gpt-image-1-mini`, `gpt-image-1.5`, `gpt-image-2`, + `gpt-image-2-2026-04-21`, or `chatgpt-image-latest`. Default: `gpt-image-1`. + """ moderation: Optional[Literal["auto", "low"]] = None """Moderation level for the generated image. Default: `auto`.""" diff --git a/src/openai/types/beta/beta_tool_param.py b/src/openai/types/beta/beta_tool_param.py index d3bef81880..640b4bfedb 100644 --- a/src/openai/types/beta/beta_tool_param.py +++ b/src/openai/types/beta/beta_tool_param.py @@ -300,7 +300,11 @@ class ImageGeneration(TypedDict, total=False): "chatgpt-image-latest", ], ] - """The image generation model to use. Default: `gpt-image-1`.""" + """The image generation model to use. + + One of `gpt-image-1`, `gpt-image-1-mini`, `gpt-image-1.5`, `gpt-image-2`, + `gpt-image-2-2026-04-21`, or `chatgpt-image-latest`. Default: `gpt-image-1`. + """ moderation: Literal["auto", "low"] """Moderation level for the generated image. Default: `auto`.""" diff --git a/src/openai/types/beta/threads/message_content.py b/src/openai/types/beta/threads/message_content.py index df6254ea9f..ad88a99b51 100644 --- a/src/openai/types/beta/threads/message_content.py +++ b/src/openai/types/beta/threads/message_content.py @@ -11,7 +11,6 @@ __all__ = ["MessageContent"] - MessageContent: TypeAlias = Annotated[ Union[ImageFileContentBlock, ImageURLContentBlock, TextContentBlock, RefusalContentBlock], PropertyInfo(discriminator="type"), diff --git a/src/openai/types/responses/tool.py b/src/openai/types/responses/tool.py index c15ee74641..69293fa845 100644 --- a/src/openai/types/responses/tool.py +++ b/src/openai/types/responses/tool.py @@ -307,7 +307,11 @@ class ImageGeneration(BaseModel): ], None, ] = None - """The image generation model to use. Default: `gpt-image-1`.""" + """The image generation model to use. + + One of `gpt-image-1`, `gpt-image-1-mini`, `gpt-image-1.5`, `gpt-image-2`, + `gpt-image-2-2026-04-21`, or `chatgpt-image-latest`. Default: `gpt-image-1`. + """ moderation: Optional[Literal["auto", "low"]] = None """Moderation level for the generated image. Default: `auto`.""" diff --git a/src/openai/types/responses/tool_param.py b/src/openai/types/responses/tool_param.py index 0b881d9c1d..9aa229cdd9 100644 --- a/src/openai/types/responses/tool_param.py +++ b/src/openai/types/responses/tool_param.py @@ -306,7 +306,11 @@ class ImageGeneration(TypedDict, total=False): "chatgpt-image-latest", ], ] - """The image generation model to use. Default: `gpt-image-1`.""" + """The image generation model to use. + + One of `gpt-image-1`, `gpt-image-1-mini`, `gpt-image-1.5`, `gpt-image-2`, + `gpt-image-2-2026-04-21`, or `chatgpt-image-latest`. Default: `gpt-image-1`. + """ moderation: Literal["auto", "low"] """Moderation level for the generated image. Default: `auto`.""" diff --git a/tests/api_resources/audio/test_speech.py b/tests/api_resources/audio/test_speech.py index 76257dc4a7..e1c56b39e8 100644 --- a/tests/api_resources/audio/test_speech.py +++ b/tests/api_resources/audio/test_speech.py @@ -26,7 +26,7 @@ class TestSpeech: def test_method_create(self, client: OpenAI, respx2_mock: MockRouter) -> None: respx2_mock.post("/audio/speech").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) speech = client.audio.speech.create( - input="string", + input="input", model="tts-1", voice="alloy", ) @@ -38,7 +38,7 @@ def test_method_create(self, client: OpenAI, respx2_mock: MockRouter) -> None: def test_method_create_with_all_params(self, client: OpenAI, respx2_mock: MockRouter) -> None: respx2_mock.post("/audio/speech").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) speech = client.audio.speech.create( - input="string", + input="input", model="tts-1", voice="alloy", instructions="instructions", @@ -55,7 +55,7 @@ def test_raw_response_create(self, client: OpenAI, respx2_mock: MockRouter) -> N respx2_mock.post("/audio/speech").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) response = client.audio.speech.with_raw_response.create( - input="string", + input="input", model="tts-1", voice="alloy", ) @@ -70,7 +70,7 @@ def test_raw_response_create(self, client: OpenAI, respx2_mock: MockRouter) -> N def test_streaming_response_create(self, client: OpenAI, respx2_mock: MockRouter) -> None: respx2_mock.post("/audio/speech").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) with client.audio.speech.with_streaming_response.create( - input="string", + input="input", model="tts-1", voice="alloy", ) as response: @@ -93,7 +93,7 @@ class TestAsyncSpeech: async def test_method_create(self, async_client: AsyncOpenAI, respx2_mock: MockRouter) -> None: respx2_mock.post("/audio/speech").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) speech = await async_client.audio.speech.create( - input="string", + input="input", model="tts-1", voice="alloy", ) @@ -105,7 +105,7 @@ async def test_method_create(self, async_client: AsyncOpenAI, respx2_mock: MockR async def test_method_create_with_all_params(self, async_client: AsyncOpenAI, respx2_mock: MockRouter) -> None: respx2_mock.post("/audio/speech").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) speech = await async_client.audio.speech.create( - input="string", + input="input", model="tts-1", voice="alloy", instructions="instructions", @@ -122,7 +122,7 @@ async def test_raw_response_create(self, async_client: AsyncOpenAI, respx2_mock: respx2_mock.post("/audio/speech").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) response = await async_client.audio.speech.with_raw_response.create( - input="string", + input="input", model="tts-1", voice="alloy", ) @@ -137,7 +137,7 @@ async def test_raw_response_create(self, async_client: AsyncOpenAI, respx2_mock: async def test_streaming_response_create(self, async_client: AsyncOpenAI, respx2_mock: MockRouter) -> None: respx2_mock.post("/audio/speech").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) async with async_client.audio.speech.with_streaming_response.create( - input="string", + input="input", model="tts-1", voice="alloy", ) as response: diff --git a/tests/api_resources/chat/test_completions.py b/tests/api_resources/chat/test_completions.py index b0b957ec4f..b6193eb1f6 100644 --- a/tests/api_resources/chat/test_completions.py +++ b/tests/api_resources/chat/test_completions.py @@ -6,7 +6,6 @@ from typing import Any, cast import pytest -import pydantic from openai import OpenAI, AsyncOpenAI from tests.utils import assert_matches_type @@ -464,23 +463,6 @@ def test_path_params_delete(self, client: OpenAI) -> None: "", ) - @parametrize - def test_method_create_disallows_pydantic(self, client: OpenAI) -> None: - class MyModel(pydantic.BaseModel): - a: str - - with pytest.raises(TypeError, match=r"You tried to pass a `BaseModel` class"): - client.chat.completions.create( - messages=[ - { - "content": "string", - "role": "system", - } - ], - model="gpt-4o", - response_format=cast(Any, MyModel), - ) - class TestAsyncCompletions: parametrize = pytest.mark.parametrize( @@ -928,20 +910,3 @@ async def test_path_params_delete(self, async_client: AsyncOpenAI) -> None: await async_client.chat.completions.with_raw_response.delete( "", ) - - @parametrize - async def test_method_create_disallows_pydantic(self, async_client: AsyncOpenAI) -> None: - class MyModel(pydantic.BaseModel): - a: str - - with pytest.raises(TypeError, match=r"You tried to pass a `BaseModel` class"): - await async_client.chat.completions.create( - messages=[ - { - "content": "string", - "role": "system", - } - ], - model="gpt-4o", - response_format=cast(Any, MyModel), - ) diff --git a/tests/api_resources/test_batches.py b/tests/api_resources/test_batches.py index 74a2365f20..f34250837c 100644 --- a/tests/api_resources/test_batches.py +++ b/tests/api_resources/test_batches.py @@ -23,7 +23,7 @@ def test_method_create(self, client: OpenAI) -> None: batch = client.batches.create( completion_window="24h", endpoint="/v1/responses", - input_file_id="string", + input_file_id="input_file_id", ) assert_matches_type(Batch, batch, path=["response"]) @@ -32,7 +32,7 @@ def test_method_create_with_all_params(self, client: OpenAI) -> None: batch = client.batches.create( completion_window="24h", endpoint="/v1/responses", - input_file_id="string", + input_file_id="input_file_id", metadata={"foo": "string"}, output_expires_after={ "anchor": "created_at", @@ -46,7 +46,7 @@ def test_raw_response_create(self, client: OpenAI) -> None: response = client.batches.with_raw_response.create( completion_window="24h", endpoint="/v1/responses", - input_file_id="string", + input_file_id="input_file_id", ) assert response.is_closed is True @@ -59,7 +59,7 @@ def test_streaming_response_create(self, client: OpenAI) -> None: with client.batches.with_streaming_response.create( completion_window="24h", endpoint="/v1/responses", - input_file_id="string", + input_file_id="input_file_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" @@ -72,14 +72,14 @@ def test_streaming_response_create(self, client: OpenAI) -> None: @parametrize def test_method_retrieve(self, client: OpenAI) -> None: batch = client.batches.retrieve( - "string", + "batch_id", ) assert_matches_type(Batch, batch, path=["response"]) @parametrize def test_raw_response_retrieve(self, client: OpenAI) -> None: response = client.batches.with_raw_response.retrieve( - "string", + "batch_id", ) assert response.is_closed is True @@ -90,7 +90,7 @@ def test_raw_response_retrieve(self, client: OpenAI) -> None: @parametrize def test_streaming_response_retrieve(self, client: OpenAI) -> None: with client.batches.with_streaming_response.retrieve( - "string", + "batch_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" @@ -115,7 +115,7 @@ def test_method_list(self, client: OpenAI) -> None: @parametrize def test_method_list_with_all_params(self, client: OpenAI) -> None: batch = client.batches.list( - after="string", + after="after", limit=0, ) assert_matches_type(SyncCursorPage[Batch], batch, path=["response"]) @@ -143,14 +143,14 @@ def test_streaming_response_list(self, client: OpenAI) -> None: @parametrize def test_method_cancel(self, client: OpenAI) -> None: batch = client.batches.cancel( - "string", + "batch_id", ) assert_matches_type(Batch, batch, path=["response"]) @parametrize def test_raw_response_cancel(self, client: OpenAI) -> None: response = client.batches.with_raw_response.cancel( - "string", + "batch_id", ) assert response.is_closed is True @@ -161,7 +161,7 @@ def test_raw_response_cancel(self, client: OpenAI) -> None: @parametrize def test_streaming_response_cancel(self, client: OpenAI) -> None: with client.batches.with_streaming_response.cancel( - "string", + "batch_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" @@ -189,7 +189,7 @@ async def test_method_create(self, async_client: AsyncOpenAI) -> None: batch = await async_client.batches.create( completion_window="24h", endpoint="/v1/responses", - input_file_id="string", + input_file_id="input_file_id", ) assert_matches_type(Batch, batch, path=["response"]) @@ -198,7 +198,7 @@ async def test_method_create_with_all_params(self, async_client: AsyncOpenAI) -> batch = await async_client.batches.create( completion_window="24h", endpoint="/v1/responses", - input_file_id="string", + input_file_id="input_file_id", metadata={"foo": "string"}, output_expires_after={ "anchor": "created_at", @@ -212,7 +212,7 @@ async def test_raw_response_create(self, async_client: AsyncOpenAI) -> None: response = await async_client.batches.with_raw_response.create( completion_window="24h", endpoint="/v1/responses", - input_file_id="string", + input_file_id="input_file_id", ) assert response.is_closed is True @@ -225,7 +225,7 @@ async def test_streaming_response_create(self, async_client: AsyncOpenAI) -> Non async with async_client.batches.with_streaming_response.create( completion_window="24h", endpoint="/v1/responses", - input_file_id="string", + input_file_id="input_file_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" @@ -238,14 +238,14 @@ async def test_streaming_response_create(self, async_client: AsyncOpenAI) -> Non @parametrize async def test_method_retrieve(self, async_client: AsyncOpenAI) -> None: batch = await async_client.batches.retrieve( - "string", + "batch_id", ) assert_matches_type(Batch, batch, path=["response"]) @parametrize async def test_raw_response_retrieve(self, async_client: AsyncOpenAI) -> None: response = await async_client.batches.with_raw_response.retrieve( - "string", + "batch_id", ) assert response.is_closed is True @@ -256,7 +256,7 @@ async def test_raw_response_retrieve(self, async_client: AsyncOpenAI) -> None: @parametrize async def test_streaming_response_retrieve(self, async_client: AsyncOpenAI) -> None: async with async_client.batches.with_streaming_response.retrieve( - "string", + "batch_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" @@ -281,7 +281,7 @@ async def test_method_list(self, async_client: AsyncOpenAI) -> None: @parametrize async def test_method_list_with_all_params(self, async_client: AsyncOpenAI) -> None: batch = await async_client.batches.list( - after="string", + after="after", limit=0, ) assert_matches_type(AsyncCursorPage[Batch], batch, path=["response"]) @@ -309,14 +309,14 @@ async def test_streaming_response_list(self, async_client: AsyncOpenAI) -> None: @parametrize async def test_method_cancel(self, async_client: AsyncOpenAI) -> None: batch = await async_client.batches.cancel( - "string", + "batch_id", ) assert_matches_type(Batch, batch, path=["response"]) @parametrize async def test_raw_response_cancel(self, async_client: AsyncOpenAI) -> None: response = await async_client.batches.with_raw_response.cancel( - "string", + "batch_id", ) assert response.is_closed is True @@ -327,7 +327,7 @@ async def test_raw_response_cancel(self, async_client: AsyncOpenAI) -> None: @parametrize async def test_streaming_response_cancel(self, async_client: AsyncOpenAI) -> None: async with async_client.batches.with_streaming_response.cancel( - "string", + "batch_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" diff --git a/tests/api_resources/test_files.py b/tests/api_resources/test_files.py index 2b820c0265..deb0e3aa61 100644 --- a/tests/api_resources/test_files.py +++ b/tests/api_resources/test_files.py @@ -72,14 +72,14 @@ def test_streaming_response_create(self, client: OpenAI) -> None: @parametrize def test_method_retrieve(self, client: OpenAI) -> None: file = client.files.retrieve( - "string", + "file_id", ) assert_matches_type(FileObject, file, path=["response"]) @parametrize def test_raw_response_retrieve(self, client: OpenAI) -> None: response = client.files.with_raw_response.retrieve( - "string", + "file_id", ) assert response.is_closed is True @@ -90,7 +90,7 @@ def test_raw_response_retrieve(self, client: OpenAI) -> None: @parametrize def test_streaming_response_retrieve(self, client: OpenAI) -> None: with client.files.with_streaming_response.retrieve( - "string", + "file_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" @@ -145,14 +145,14 @@ def test_streaming_response_list(self, client: OpenAI) -> None: @parametrize def test_method_delete(self, client: OpenAI) -> None: file = client.files.delete( - "string", + "file_id", ) assert_matches_type(FileDeleted, file, path=["response"]) @parametrize def test_raw_response_delete(self, client: OpenAI) -> None: response = client.files.with_raw_response.delete( - "string", + "file_id", ) assert response.is_closed is True @@ -163,7 +163,7 @@ def test_raw_response_delete(self, client: OpenAI) -> None: @parametrize def test_streaming_response_delete(self, client: OpenAI) -> None: with client.files.with_streaming_response.delete( - "string", + "file_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" @@ -183,9 +183,9 @@ def test_path_params_delete(self, client: OpenAI) -> None: @parametrize @pytest.mark.respx2(base_url=base_url) def test_method_content(self, client: OpenAI, respx2_mock: MockRouter) -> None: - respx2_mock.get("/files/string/content").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) + respx2_mock.get("/files/file_id/content").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) file = client.files.content( - "string", + "file_id", ) assert isinstance(file, _legacy_response.HttpxBinaryResponseContent) assert file.json() == {"foo": "bar"} @@ -193,10 +193,10 @@ def test_method_content(self, client: OpenAI, respx2_mock: MockRouter) -> None: @parametrize @pytest.mark.respx2(base_url=base_url) def test_raw_response_content(self, client: OpenAI, respx2_mock: MockRouter) -> None: - respx2_mock.get("/files/string/content").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) + respx2_mock.get("/files/file_id/content").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) response = client.files.with_raw_response.content( - "string", + "file_id", ) assert response.is_closed is True @@ -207,9 +207,9 @@ def test_raw_response_content(self, client: OpenAI, respx2_mock: MockRouter) -> @parametrize @pytest.mark.respx2(base_url=base_url) def test_streaming_response_content(self, client: OpenAI, respx2_mock: MockRouter) -> None: - respx2_mock.get("/files/string/content").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) + respx2_mock.get("/files/file_id/content").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) with client.files.with_streaming_response.content( - "string", + "file_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" @@ -231,7 +231,7 @@ def test_path_params_content(self, client: OpenAI) -> None: def test_method_retrieve_content(self, client: OpenAI) -> None: with pytest.warns(DeprecationWarning): file = client.files.retrieve_content( - "string", + "file_id", ) assert_matches_type(str, file, path=["response"]) @@ -240,7 +240,7 @@ def test_method_retrieve_content(self, client: OpenAI) -> None: def test_raw_response_retrieve_content(self, client: OpenAI) -> None: with pytest.warns(DeprecationWarning): response = client.files.with_raw_response.retrieve_content( - "string", + "file_id", ) assert response.is_closed is True @@ -252,7 +252,7 @@ def test_raw_response_retrieve_content(self, client: OpenAI) -> None: def test_streaming_response_retrieve_content(self, client: OpenAI) -> None: with pytest.warns(DeprecationWarning): with client.files.with_streaming_response.retrieve_content( - "string", + "file_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" @@ -325,14 +325,14 @@ async def test_streaming_response_create(self, async_client: AsyncOpenAI) -> Non @parametrize async def test_method_retrieve(self, async_client: AsyncOpenAI) -> None: file = await async_client.files.retrieve( - "string", + "file_id", ) assert_matches_type(FileObject, file, path=["response"]) @parametrize async def test_raw_response_retrieve(self, async_client: AsyncOpenAI) -> None: response = await async_client.files.with_raw_response.retrieve( - "string", + "file_id", ) assert response.is_closed is True @@ -343,7 +343,7 @@ async def test_raw_response_retrieve(self, async_client: AsyncOpenAI) -> None: @parametrize async def test_streaming_response_retrieve(self, async_client: AsyncOpenAI) -> None: async with async_client.files.with_streaming_response.retrieve( - "string", + "file_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" @@ -398,14 +398,14 @@ async def test_streaming_response_list(self, async_client: AsyncOpenAI) -> None: @parametrize async def test_method_delete(self, async_client: AsyncOpenAI) -> None: file = await async_client.files.delete( - "string", + "file_id", ) assert_matches_type(FileDeleted, file, path=["response"]) @parametrize async def test_raw_response_delete(self, async_client: AsyncOpenAI) -> None: response = await async_client.files.with_raw_response.delete( - "string", + "file_id", ) assert response.is_closed is True @@ -416,7 +416,7 @@ async def test_raw_response_delete(self, async_client: AsyncOpenAI) -> None: @parametrize async def test_streaming_response_delete(self, async_client: AsyncOpenAI) -> None: async with async_client.files.with_streaming_response.delete( - "string", + "file_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" @@ -436,9 +436,9 @@ async def test_path_params_delete(self, async_client: AsyncOpenAI) -> None: @parametrize @pytest.mark.respx2(base_url=base_url) async def test_method_content(self, async_client: AsyncOpenAI, respx2_mock: MockRouter) -> None: - respx2_mock.get("/files/string/content").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) + respx2_mock.get("/files/file_id/content").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) file = await async_client.files.content( - "string", + "file_id", ) assert isinstance(file, _legacy_response.HttpxBinaryResponseContent) assert file.json() == {"foo": "bar"} @@ -446,10 +446,10 @@ async def test_method_content(self, async_client: AsyncOpenAI, respx2_mock: Mock @parametrize @pytest.mark.respx2(base_url=base_url) async def test_raw_response_content(self, async_client: AsyncOpenAI, respx2_mock: MockRouter) -> None: - respx2_mock.get("/files/string/content").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) + respx2_mock.get("/files/file_id/content").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) response = await async_client.files.with_raw_response.content( - "string", + "file_id", ) assert response.is_closed is True @@ -460,9 +460,9 @@ async def test_raw_response_content(self, async_client: AsyncOpenAI, respx2_mock @parametrize @pytest.mark.respx2(base_url=base_url) async def test_streaming_response_content(self, async_client: AsyncOpenAI, respx2_mock: MockRouter) -> None: - respx2_mock.get("/files/string/content").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) + respx2_mock.get("/files/file_id/content").mock(return_value=httpx2.Response(200, json={"foo": "bar"})) async with async_client.files.with_streaming_response.content( - "string", + "file_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" @@ -484,7 +484,7 @@ async def test_path_params_content(self, async_client: AsyncOpenAI) -> None: async def test_method_retrieve_content(self, async_client: AsyncOpenAI) -> None: with pytest.warns(DeprecationWarning): file = await async_client.files.retrieve_content( - "string", + "file_id", ) assert_matches_type(str, file, path=["response"]) @@ -493,7 +493,7 @@ async def test_method_retrieve_content(self, async_client: AsyncOpenAI) -> None: async def test_raw_response_retrieve_content(self, async_client: AsyncOpenAI) -> None: with pytest.warns(DeprecationWarning): response = await async_client.files.with_raw_response.retrieve_content( - "string", + "file_id", ) assert response.is_closed is True @@ -505,7 +505,7 @@ async def test_raw_response_retrieve_content(self, async_client: AsyncOpenAI) -> async def test_streaming_response_retrieve_content(self, async_client: AsyncOpenAI) -> None: with pytest.warns(DeprecationWarning): async with async_client.files.with_streaming_response.retrieve_content( - "string", + "file_id", ) as response: assert not response.is_closed assert response.http_request.headers.get("X-Stainless-Lang") == "python" diff --git a/tests/api_resources/test_responses.py b/tests/api_resources/test_responses.py index 2e97ca9416..22a5c008b4 100644 --- a/tests/api_resources/test_responses.py +++ b/tests/api_resources/test_responses.py @@ -9,7 +9,6 @@ from openai import OpenAI, AsyncOpenAI from tests.utils import assert_matches_type -from openai._utils import assert_signatures_in_sync from openai.types.responses import ( Response, CompactedResponse, @@ -452,17 +451,6 @@ def test_streaming_response_compact(self, client: OpenAI) -> None: assert cast(Any, http_response.is_closed) is True -@pytest.mark.parametrize("sync", [True, False], ids=["sync", "async"]) -def test_parse_method_in_sync(sync: bool, client: OpenAI, async_client: AsyncOpenAI) -> None: - checking_client: OpenAI | AsyncOpenAI = client if sync else async_client - - assert_signatures_in_sync( - checking_client.responses.create, - checking_client.responses.parse, - exclude_params={"stream", "tools"}, - ) - - class TestAsyncResponses: parametrize = pytest.mark.parametrize( "async_client", [False, True, {"http_client": "aiohttp"}], indirect=True, ids=["loose", "strict", "aiohttp"] diff --git a/tests/lib/chat/test_create_validation.py b/tests/lib/chat/test_create_validation.py new file mode 100644 index 0000000000..f00db1a4f6 --- /dev/null +++ b/tests/lib/chat/test_create_validation.py @@ -0,0 +1,52 @@ +from __future__ import annotations + +from typing import Any, cast + +import pytest +import pydantic + +from openai import OpenAI, AsyncOpenAI + + +class TestCompletions: + parametrize = pytest.mark.parametrize("client", [False, True], indirect=True, ids=["loose", "strict"]) + + @parametrize + def test_method_create_disallows_pydantic(self, client: OpenAI) -> None: + class MyModel(pydantic.BaseModel): + a: str + + with pytest.raises(TypeError, match=r"You tried to pass a `BaseModel` class"): + client.chat.completions.create( + messages=[ + { + "content": "string", + "role": "system", + } + ], + model="gpt-4o", + response_format=cast(Any, MyModel), + ) + + +class TestAsyncCompletions: + parametrize = pytest.mark.parametrize( + "async_client", [False, True, {"http_client": "aiohttp"}], indirect=True, ids=["loose", "strict", "aiohttp"] + ) + + @parametrize + async def test_method_create_disallows_pydantic(self, async_client: AsyncOpenAI) -> None: + class MyModel(pydantic.BaseModel): + a: str + + with pytest.raises(TypeError, match=r"You tried to pass a `BaseModel` class"): + await async_client.chat.completions.create( + messages=[ + { + "content": "string", + "role": "system", + } + ], + model="gpt-4o", + response_format=cast(Any, MyModel), + ) diff --git a/tests/lib/responses/test_responses.py b/tests/lib/responses/test_responses.py index 43879942b8..c970c53a77 100644 --- a/tests/lib/responses/test_responses.py +++ b/tests/lib/responses/test_responses.py @@ -83,6 +83,17 @@ def test_stream_method_definition_in_sync(sync: bool, client: OpenAI, async_clie ) +@pytest.mark.parametrize("sync", [True, False], ids=["sync", "async"]) +def test_parse_method_in_sync(sync: bool, client: OpenAI, async_client: AsyncOpenAI) -> None: + checking_client: OpenAI | AsyncOpenAI = client if sync else async_client + + assert_signatures_in_sync( + checking_client.responses.create, + checking_client.responses.parse, + exclude_params={"stream", "tools"}, + ) + + @pytest.mark.parametrize("sync", [True, False], ids=["sync", "async"]) def test_parse_method_definition_in_sync(sync: bool, client: OpenAI, async_client: AsyncOpenAI) -> None: checking_client: OpenAI | AsyncOpenAI = client if sync else async_client diff --git a/tests/lib/test_azure_provider_diagnostics.py b/tests/lib/test_azure_provider_diagnostics.py new file mode 100644 index 0000000000..4b9e201867 --- /dev/null +++ b/tests/lib/test_azure_provider_diagnostics.py @@ -0,0 +1,192 @@ +from __future__ import annotations + +import asyncio +import inspect +import traceback +from typing import Any, Callable, NoReturn, NamedTuple +from typing_extensions import override + +import httpx2 +import pytest + +from openai import AzureOpenAI, AsyncAzureOpenAI + +FAKE_TOKEN = "fake-azure-provider-token" +ERROR_MESSAGE = "Expected `azure_ad_token_provider` argument to return a non-empty string." +PROVIDER_MODES = ["sync", "async-direct", "async-coroutine", "async-awaitable"] + + +class FakeAccessToken(NamedTuple): + token: str + expires_on: int + + +class UninspectableToken: + def __bool__(self) -> NoReturn: + raise TypeError(FAKE_TOKEN) + + @override + def __str__(self) -> NoReturn: + raise AssertionError(FAKE_TOKEN) + + @override + def __repr__(self) -> NoReturn: + raise AssertionError(FAKE_TOKEN) + + +class OrdinaryToken(str): + pass + + +class ReformattedToken(str): + @override + def __format__(self, format_spec: str) -> str: + return "fake-altered-token" + + +class UnformattableToken(str): + @override + def __format__(self, format_spec: str) -> NoReturn: + raise TypeError(FAKE_TOKEN) + + +class UninspectableString(UnformattableToken): + def __bool__(self) -> NoReturn: + raise TypeError(FAKE_TOKEN) + + @override + def __len__(self) -> NoReturn: + raise TypeError(FAKE_TOKEN) + + @override + def __str__(self) -> NoReturn: + raise TypeError(FAKE_TOKEN) + + +class WebSocketConnectReached(Exception): + pass + + +@pytest.fixture(autouse=True) +def offline_environment(monkeypatch: pytest.MonkeyPatch) -> None: + for name in ("AZURE_OPENAI_API_KEY", "AZURE_OPENAI_AD_TOKEN", "OPENAI_API_KEY"): + monkeypatch.delenv(name, raising=False) + + def unexpected_connection(*_args: Any, **_kwargs: Any) -> NoReturn: + pytest.fail("Azure provider diagnostics must not open a network connection") + + monkeypatch.setattr("socket.socket.connect", unexpected_connection) + monkeypatch.setattr("socket.socket.connect_ex", unexpected_connection) + + +def make_provider(mode: str, value: object) -> Callable[[], Any]: + async def coroutine() -> object: + return value + + def awaitable() -> asyncio.Future[object]: + future: asyncio.Future[object] = asyncio.get_running_loop().create_future() + future.set_result(value) + return future + + if mode == "async-coroutine": + return coroutine + if mode == "async-awaitable": + return awaitable + return lambda: value + + +def make_client(mode: str, value: object, requests: list[httpx2.Request]) -> Any: + def send(request: httpx2.Request) -> httpx2.Response: + requests.append(request) + return httpx2.Response(200, json={"data": []}) + + transport = httpx2.MockTransport(send) + http_client = httpx2.Client(transport=transport) if mode == "sync" else httpx2.AsyncClient(transport=transport) + cls: Any = AzureOpenAI if mode == "sync" else AsyncAzureOpenAI + return cls( + azure_endpoint="https://azure.test", + api_version="2024-02-01", + azure_ad_token_provider=make_provider(mode, value), + http_client=http_client, + max_retries=0, + ) + + +async def resolve(value: Any) -> Any: + return await value if inspect.isawaitable(value) else value + + +@pytest.mark.parametrize("mode", PROVIDER_MODES) +@pytest.mark.parametrize("entrypoint", ["http", "realtime-config", "realtime", "beta-realtime"]) +@pytest.mark.parametrize( + "value", + [ + pytest.param({"access_token": FAKE_TOKEN}, id="dict"), + pytest.param(FakeAccessToken(FAKE_TOKEN, 0), id="access-token"), + pytest.param(UninspectableToken(), id="uninspectable-object"), + pytest.param("", id="empty-string"), + pytest.param(UninspectableString(""), id="empty-string-subclass"), + pytest.param(None, id="none"), + ], +) +async def test_invalid_provider_result_is_value_free(mode: str, entrypoint: str, value: object) -> None: + requests: list[httpx2.Request] = [] + client = make_client(mode, value, requests) + try: + with pytest.raises(ValueError) as exc_info: + if entrypoint == "http": + await resolve(client.models.list()) + elif entrypoint == "realtime-config": + await resolve(client._configure_realtime("test-model", {})) + else: + resource = client.realtime if entrypoint == "realtime" else client.beta.realtime + await resolve(resource.connect(model="test-model").enter()) + + error = exc_info.value + assert str(error) == ERROR_MESSAGE + assert FAKE_TOKEN not in repr(error) + assert FAKE_TOKEN not in "".join(traceback.format_exception(type(error), error, error.__traceback__)) + assert error.__cause__ is None + assert error.__context__ is None + assert requests == [] + finally: + await resolve(client.close()) + + +@pytest.mark.parametrize("mode", PROVIDER_MODES) +@pytest.mark.parametrize( + "value", + [ + pytest.param(FAKE_TOKEN, id="string"), + pytest.param(OrdinaryToken(FAKE_TOKEN), id="ordinary-subclass"), + pytest.param(ReformattedToken(FAKE_TOKEN), id="reformatted-subclass"), + pytest.param(UnformattableToken(FAKE_TOKEN), id="unformattable-subclass"), + pytest.param(UninspectableString(FAKE_TOKEN), id="uninspectable-subclass"), + ], +) +async def test_nonempty_provider_result_remains_usable(mode: str, value: str, monkeypatch: pytest.MonkeyPatch) -> None: + websocket_headers: list[dict[str, str]] = [] + + def connect(_url: str, **kwargs: Any) -> NoReturn: + websocket_headers.append(kwargs["additional_headers"]) + raise WebSocketConnectReached + + monkeypatch.setattr("websockets.sync.client.connect", connect) + monkeypatch.setattr("openai.lib._azure_websocket._AzureWebSocketConnect", connect) + requests: list[httpx2.Request] = [] + client = make_client(mode, value, requests) + try: + await resolve(client.models.list()) + assert len(requests) == 1 + assert requests[0].headers["Authorization"] == f"Bearer {FAKE_TOKEN}" + _, headers = await resolve(client._configure_realtime("test-model", {})) + assert headers == {"Authorization": f"Bearer {FAKE_TOKEN}"} + token = await resolve(client._get_azure_ad_token()) + assert type(token) is str + assert token == FAKE_TOKEN + for resource in (client.realtime, client.beta.realtime): + with pytest.raises(WebSocketConnectReached): + await resolve(resource.connect(model="test-model").enter()) + assert [headers["Authorization"] for headers in websocket_headers] == [f"Bearer {FAKE_TOKEN}"] * 2 + finally: + await resolve(client.close()) diff --git a/tests/lib/test_websocket_redirects.py b/tests/lib/test_websocket_redirects.py new file mode 100644 index 0000000000..2ae84e6bfb --- /dev/null +++ b/tests/lib/test_websocket_redirects.py @@ -0,0 +1,226 @@ +from __future__ import annotations + +import asyncio +from typing import Any, Callable +from unittest.mock import Mock, AsyncMock + +import httpx2 +import pytest +from websockets.uri import WebSocketURI +from websockets.http11 import Response +from websockets.exceptions import InvalidStatus, SecurityError +from websockets.asyncio.client import connect +from websockets.datastructures import Headers, HeadersLike + +from openai import OpenAI, AsyncOpenAI, AsyncAzureOpenAI +from openai.lib._websocket import _WebSocketConnect +from openai.types.websocket_reconnection import ReconnectingEvent + +RESOURCES = ["realtime", "beta.realtime", "responses", "beta.responses"] +RECONNECTING_RESOURCES = [name for name in RESOURCES if name != "beta.realtime"] +EXTRA_HEADERS = {"api-key": "fake-key", "Cookie": "fake-cookie", "X-Custom": "fake-private-header"} +FOLLOWS_REDIRECTS = hasattr(connect, "process_redirect") + + +def unexpected_http(_request: httpx2.Request) -> httpx2.Response: + pytest.fail("Unexpected HTTP request") + + +def async_http_client() -> httpx2.AsyncClient: + return httpx2.AsyncClient(transport=httpx2.MockTransport(unexpected_http)) + + +def resource(client: Any, name: str) -> Any: + for part in name.split("."): + client = getattr(client, part) + return client + + +def options(name: str) -> dict[str, Any]: + return {"extra_headers": EXTRA_HEADERS, **({"model": "fake-model"} if name.endswith("realtime") else {})} + + +def redirect_error(location: str) -> InvalidStatus: + return InvalidStatus(Response(302, "Found", Headers({"Location": location}))) + + +def no_proxy(_uri: WebSocketURI) -> None: + return None + + +def reconnect(_event: ReconnectingEvent) -> None: + return None + + +class Handshakes: + """Exercise the installed connector's handshake loop without opening sockets.""" + + def __init__(self, monkeypatch: pytest.MonkeyPatch, redirects: list[str | None]) -> None: + self.redirects = iter(redirects) + self.attempts: list[tuple[WebSocketURI, Headers]] = [] + self.sent: list[tuple[WebSocketURI, str]] = [] + monkeypatch.setattr(asyncio.get_running_loop(), "create_connection", self.create_connection) + # websockets 15 can discover proxies before reaching create_connection. + monkeypatch.setattr("websockets.asyncio.client.get_proxy", no_proxy, raising=False) + + async def create_connection(self, factory: Callable[[], Any], **_kwargs: Any) -> tuple[Mock, Mock]: + protocol = factory().protocol + # The Sans-I/O protocol renamed wsuri to uri in websockets 15. + uri: WebSocketURI = protocol.uri if hasattr(protocol, "uri") else protocol.wsuri + websocket = Mock() + + async def handshake(headers: HeadersLike | None, _user_agent: str | None) -> None: + self.attempts.append((uri, Headers(headers or {}))) + location = next(self.redirects) + if location is not None: + raise redirect_error(location) + + async def send(data: str) -> None: + self.sent.append((uri, data)) + + websocket.handshake = AsyncMock(side_effect=handshake) + websocket.send = AsyncMock(side_effect=send) + websocket.close = AsyncMock() + return websocket.transport, websocket + + +@pytest.mark.parametrize("name", RESOURCES) +@pytest.mark.parametrize( + "base,target,allowed", + [ + ("wss://origin.test", None, True), + ("wss://origin.test", "/final", True), + ("wss://origin.test", "wss://ORIGIN.test:443/final", True), + ("wss://origin.test", "wss://other.test/final", False), + ("wss://origin.test", "//other.test/final", False), + ("wss://origin.test", "wss://origin.test:444/final", False), + ("wss://origin.test", "ws://origin.test/final", False), + ("ws://origin.test", "wss://origin.test/final", False), + ], +) +async def test_async_websocket_redirects( + monkeypatch: pytest.MonkeyPatch, name: str, base: str, target: str | None, allowed: bool +) -> None: + handshakes = Handshakes(monkeypatch, [target, None]) + succeeds = allowed and (target is None or FOLLOWS_REDIRECTS) + async with AsyncOpenAI( + api_key="fake-entra-token", + websocket_base_url=base, + http_client=async_http_client(), + ) as client: + manager = resource(client, name).connect(**options(name)) + if name in RECONNECTING_RESOURCES: + manager.send({"type": "response.create"}) + if succeeds: + async with manager as connection: + await connection.send({"type": "response.create"}) + else: + expected = SecurityError if FOLLOWS_REDIRECTS else InvalidStatus + with pytest.raises(expected): + async with manager: + pytest.fail("Unexpected connection") + + assert len(handshakes.attempts) == (2 if succeeds and target else 1) + for uri, headers in handshakes.attempts: + assert (uri.secure, uri.host, uri.port) == ( + base.startswith("wss:"), + "origin.test", + 443 if base.startswith("wss:") else 80, + ) + assert headers["Authorization"] == "Bearer fake-entra-token" + for key, value in EXTRA_HEADERS.items(): + assert headers[key] == value + assert bool(handshakes.sent) is succeeds + assert all(uri.host == "origin.test" for uri, _ in handshakes.sent) + + +@pytest.mark.skipif(not FOLLOWS_REDIRECTS, reason="No automatic handshake redirects") +@pytest.mark.parametrize("name", RESOURCES) +async def test_later_cross_origin_redirect_is_rejected(monkeypatch: pytest.MonkeyPatch, name: str) -> None: + handshakes = Handshakes(monkeypatch, ["/intermediate", "wss://other.test/final", None]) + async with AsyncOpenAI( + api_key="fake-key", websocket_base_url="wss://origin.test", http_client=async_http_client() + ) as client: + with pytest.raises(SecurityError): + async with resource(client, name).connect(**options(name)): + pytest.fail("Unexpected connection") + assert len(handshakes.attempts) == 2 + assert all(uri.host == "origin.test" for uri, _ in handshakes.attempts) + assert handshakes.sent == [] + + +@pytest.mark.parametrize("name", RECONNECTING_RESOURCES) +@pytest.mark.parametrize( + "target", ["/final", "wss://other.test/final", "wss://origin.test:444/final", "ws://origin.test/final"] +) +async def test_async_websocket_reconnect_redirects(monkeypatch: pytest.MonkeyPatch, name: str, target: str) -> None: + handshakes = Handshakes(monkeypatch, [None, target, None]) + succeeds = target == "/final" and FOLLOWS_REDIRECTS + async with AsyncOpenAI( + api_key="fake-key", websocket_base_url="wss://origin.test", http_client=async_http_client() + ) as client: + manager = resource(client, name).connect( + **options(name), on_reconnecting=reconnect, initial_delay=0, max_retries=1 + ) + async with manager as connection: + connection._send_queue.enqueue("fake-queued-message") + assert await connection._reconnect(RuntimeError("fake disconnect")) is succeeds + assert (connection._send_queue._bytes == 0) is succeeds + + assert len(handshakes.attempts) == (3 if succeeds else 2) + assert all((uri.secure, uri.host, uri.port) == (True, "origin.test", 443) for uri, _ in handshakes.attempts) + assert [data for _, data in handshakes.sent] == (["fake-queued-message"] if succeeds else []) + for _, headers in handshakes.attempts: + assert headers["Authorization"] == "Bearer fake-key" + + +@pytest.mark.parametrize("name", ["realtime", "beta.realtime"]) +@pytest.mark.parametrize("target", ["/final", "wss://other.test/final"]) +async def test_async_azure_guard_is_preserved(monkeypatch: pytest.MonkeyPatch, name: str, target: str) -> None: + handshakes = Handshakes(monkeypatch, [target, None]) + succeeds = target == "/final" and FOLLOWS_REDIRECTS + async with AsyncAzureOpenAI( + api_key="fake-key", + azure_endpoint="https://origin.test", + api_version="2024-01-01", + http_client=async_http_client(), + ) as client: + manager = resource(client, name).connect(model="fake-model") + if succeeds: + async with manager: + pass + else: + expected = SecurityError if FOLLOWS_REDIRECTS else InvalidStatus + with pytest.raises(expected): + async with manager: + pytest.fail("Unexpected connection") + assert len(handshakes.attempts) == (2 if succeeds else 1) + assert all(uri.host == "origin.test" and headers["api-key"] == "fake-key" for uri, headers in handshakes.attempts) + + +@pytest.mark.parametrize("name", RESOURCES) +def test_sync_websocket_connector_is_unchanged(monkeypatch: pytest.MonkeyPatch, name: str) -> None: + websocket = Mock() + connect_mock = Mock(return_value=websocket) + monkeypatch.setattr("websockets.sync.client.connect", connect_mock) + with OpenAI( + api_key="fake-key", + websocket_base_url="wss://origin.test", + http_client=httpx2.Client(transport=httpx2.MockTransport(unexpected_http)), + ) as client: + with resource(client, name).connect(**options(name)): + pass + error = redirect_error("wss://other.test/final") + connect_mock.side_effect = error + with pytest.raises(InvalidStatus) as caught: + with resource(client, name).connect(**options(name)): + pytest.fail("Unexpected connection") + assert caught.value is error + assert connect_mock.call_count == 2 + assert connect_mock.call_args.kwargs["additional_headers"]["Authorization"] == "Bearer fake-key" + + +@pytest.mark.skipif(not FOLLOWS_REDIRECTS, reason="No automatic handshake redirects") +def test_non_redirect_error_is_preserved() -> None: + error = InvalidStatus(Response(401, "Unauthorized", Headers())) + assert _WebSocketConnect("wss://origin.test/realtime").process_redirect(error) is error diff --git a/tests/test_debug_logging.py b/tests/test_debug_logging.py index 2f7c033c67..3ba51161c4 100644 --- a/tests/test_debug_logging.py +++ b/tests/test_debug_logging.py @@ -246,7 +246,7 @@ async def test_websocket_connection_metadata( kwargs["model"] = "fake-model" with ( caplog.at_level(logging.DEBUG, logger="openai"), - patch("websockets." + ("asyncio" if asynchronous else "sync") + ".client.connect", connect), + patch("openai.lib._websocket._WebSocketConnect" if asynchronous else "websockets.sync.client.connect", connect), ): if asynchronous: async with AsyncOpenAI( diff --git a/tests/test_tls_hostname.py b/tests/test_tls_hostname.py new file mode 100644 index 0000000000..6bcb5497f4 --- /dev/null +++ b/tests/test_tls_hostname.py @@ -0,0 +1,258 @@ +from __future__ import annotations + +import ssl +import socket +import datetime +import importlib +import threading +import importlib.util +from typing import Any, cast +from pathlib import Path +from http.server import ThreadingHTTPServer, BaseHTTPRequestHandler +from collections.abc import Iterator +from typing_extensions import override + +import pytest + +from openai import OpenAI, AsyncOpenAI, APIConnectionError, DefaultAioHttpClient +from openai.auth import x509_workload_identity + +HOSTS = ["sdk-host.test", "sdk_host.test", "sub_name.sdk_host.test"] +HTTP_LIBRARIES = ["httpx2", "httpx"] + + +def http_library(name: str) -> Any: + if importlib.util.find_spec(name) is None: + pytest.skip(f"{name} is not installed") + return cast(Any, importlib.import_module(name)) + + +@pytest.mark.parametrize("library", HTTP_LIBRARIES) +@pytest.mark.parametrize("host", HOSTS) +@pytest.mark.parametrize("explicit_sni", [None, "private-pki.test"]) +@pytest.mark.parametrize("x509", [False, True]) +def test_sync_request_preserves_tls_hostname(library: str, host: str, explicit_sni: str | None, x509: bool) -> None: + http = http_library(library) + requests: list[Any] = [] + + def handler(request: Any) -> Any: + if request.url.host == "mtls.auth.openai.com": + return http.Response(200, json={"access_token": "fake-token", "expires_in": 3600}) + requests.append(request) + return http.Response(200, json={"object": "list", "data": []}) + + def hook(request: Any) -> None: + if explicit_sni is not None: + request.extensions["sni_hostname"] = explicit_sni + + identity = x509_workload_identity(identity_provider_id="fake-provider", service_account_id="fake-account") + with OpenAI( + api_key=None if x509 else "fake-api-key", + workload_identity=identity if x509 else None, + base_url=f"https://{host}/v1", + http_client=http.Client( + transport=http.MockTransport(handler), event_hooks={"request": [hook]}, trust_env=False + ), + max_retries=0, + ) as client: + response = client.get( + f"https://{host}/v1/models?configured=1", cast_to=http.Response, options={"params": {"request": "2"}} + ) + assert response.json()["object"] == "list" + + assert len(requests) == 1 + assert requests[0].url.host == host + assert requests[0].headers["host"] == host + assert requests[0].url.params["configured"] == "1" + assert requests[0].url.params["request"] == "2" + assert requests[0].extensions.get("sni_hostname") == explicit_sni + assert "timeout" in requests[0].extensions + + +@pytest.mark.parametrize("library", HTTP_LIBRARIES) +@pytest.mark.parametrize("host", HOSTS) +@pytest.mark.parametrize("explicit_sni", [None, "private-pki.test"]) +@pytest.mark.parametrize("x509", [False, True]) +async def test_async_request_preserves_tls_hostname( + library: str, host: str, explicit_sni: str | None, x509: bool +) -> None: + http = http_library(library) + requests: list[Any] = [] + + async def handler(request: Any) -> Any: + if request.url.host == "mtls.auth.openai.com": + return http.Response(200, json={"access_token": "fake-token", "expires_in": 3600}) + requests.append(request) + return http.Response(200, json={"object": "list", "data": []}) + + async def hook(request: Any) -> None: + if explicit_sni is not None: + request.extensions["sni_hostname"] = explicit_sni + + identity = x509_workload_identity(identity_provider_id="fake-provider", service_account_id="fake-account") + async with AsyncOpenAI( + api_key=None if x509 else "fake-api-key", + workload_identity=identity if x509 else None, + base_url=f"https://{host}/v1", + http_client=http.AsyncClient( + transport=http.MockTransport(handler), event_hooks={"request": [hook]}, trust_env=False + ), + max_retries=0, + ) as client: + response = await client.get( + f"https://{host}/v1/models?configured=1", cast_to=http.Response, options={"params": {"request": "2"}} + ) + assert response.json()["object"] == "list" + + assert len(requests) == 1 + assert requests[0].url.host == host + assert requests[0].headers["host"] == host + assert requests[0].url.params["configured"] == "1" + assert requests[0].url.params["request"] == "2" + assert requests[0].extensions.get("sni_hostname") == explicit_sni + assert "timeout" in requests[0].extensions + + +@pytest.fixture +def tls_server(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Iterator[tuple[int, ssl.SSLContext, list[str]]]: + # Generate a short-lived, test-only certificate and key; never check in private keys. + pytest.importorskip("cryptography") + from cryptography import x509 + from cryptography.x509.oid import NameOID + from cryptography.hazmat.primitives import hashes, serialization + from cryptography.hazmat.primitives.asymmetric import rsa + + key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + name = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, HOSTS[0])]) + now = datetime.datetime.now(datetime.timezone.utc) + cert = ( + x509.CertificateBuilder() + .subject_name(name) + .issuer_name(name) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(now - datetime.timedelta(minutes=1)) + .not_valid_after(now + datetime.timedelta(days=1)) + .add_extension(x509.SubjectAlternativeName([x509.DNSName(HOSTS[0])]), critical=False) + .sign(key, hashes.SHA256()) + ) + cert_path = tmp_path / "server.pem" + key_path = tmp_path / "server.key" + cert_path.write_bytes(cert.public_bytes(serialization.Encoding.PEM)) + key_path.write_bytes( + key.private_bytes(serialization.Encoding.PEM, serialization.PrivateFormat.PKCS8, serialization.NoEncryption()) + ) + server_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + server_context.minimum_version = ssl.TLSVersion.TLSv1_2 + server_context.load_cert_chain(cert_path, key_path) + client_context = ssl.create_default_context(cafile=str(cert_path)) + requests: list[str] = [] + + class Handler(BaseHTTPRequestHandler): + def do_GET(self) -> None: + requests.append(self.headers["Host"]) + body = b'{"object":"list","data":[]}' + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + @override + def log_message(self, *_args: object, **_kwargs: object) -> None: + pass + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + server.socket = server_context.wrap_socket(server.socket, server_side=True) + original_getaddrinfo = socket.getaddrinfo + + def getaddrinfo(host: Any, port: Any, *args: Any, **kwargs: Any) -> Any: + # Never resolve or connect to an external endpoint, even if the client regresses. + assert host in (HOSTS[0], HOSTS[1], HOSTS[0].encode(), HOSTS[1].encode(), "127.0.0.1", b"127.0.0.1") + assert int(port) == server.server_port + return original_getaddrinfo("127.0.0.1", port, *args, **kwargs) + + monkeypatch.setattr(socket, "getaddrinfo", getaddrinfo) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield server.server_port, client_context, requests + finally: + server.shutdown() + server.server_close() + thread.join() + + +def assert_certificate_error(error: BaseException) -> None: + cause: BaseException | None = error + while cause is not None: + if isinstance(cause, ssl.SSLCertVerificationError): + return + cause = cause.__cause__ or cause.__context__ + pytest.fail("Expected certificate hostname verification to fail") + + +@pytest.mark.parametrize("explicit_sni", [False, True]) +@pytest.mark.parametrize("library", HTTP_LIBRARIES) +def test_sync_tls_verifies_configured_hostname( + library: str, explicit_sni: bool, tls_server: tuple[int, ssl.SSLContext, list[str]] +) -> None: + http = http_library(library) + port, context, requests = tls_server + + def hook(request: Any) -> None: + if explicit_sni: + request.extensions["sni_hostname"] = HOSTS[0] + + for host in HOSTS[:2]: + with OpenAI( + api_key="fake-api-key", + base_url=f"https://{host}:{port}/v1", + http_client=http.Client(verify=context, trust_env=False, event_hooks={"request": [hook]}), + max_retries=0, + ) as client: + if host == HOSTS[0] or explicit_sni: + assert client.models.list().object == "list" + else: + with pytest.raises(APIConnectionError) as exc: + client.models.list() + assert_certificate_error(exc.value) + assert requests == [f"{host}:{port}" for host in HOSTS[: 2 if explicit_sni else 1]] + + +@pytest.mark.parametrize("explicit_sni", [False, True]) +@pytest.mark.parametrize("library", [*HTTP_LIBRARIES, "aiohttp", "httpx_aiohttp"]) +async def test_async_tls_verifies_configured_hostname( + library: str, explicit_sni: bool, tls_server: tuple[int, ssl.SSLContext, list[str]] +) -> None: + port, context, requests = tls_server + + async def hook(request: Any) -> None: + if explicit_sni: + request.extensions["sni_hostname"] = HOSTS[0] + + for host in HOSTS[:2]: + if library == "aiohttp": + pytest.importorskip("aiohttp") + http_client = DefaultAioHttpClient(verify=context, trust_env=False, event_hooks={"request": [hook]}) + elif library == "httpx_aiohttp": + http_client = http_library(library).HttpxAiohttpClient( + verify=context, trust_env=False, event_hooks={"request": [hook]} + ) + else: + http_client = http_library(library).AsyncClient( + verify=context, trust_env=False, event_hooks={"request": [hook]} + ) + async with AsyncOpenAI( + api_key="fake-api-key", + base_url=f"https://{host}:{port}/v1", + http_client=http_client, + max_retries=0, + ) as client: + if host == HOSTS[0] or explicit_sni: + assert (await client.models.list()).object == "list" + else: + with pytest.raises(APIConnectionError) as exc: + await client.models.list() + assert_certificate_error(exc.value) + assert requests == [f"{host}:{port}" for host in HOSTS[: 2 if explicit_sni else 1]]