From ec7430abf81f2c6ff9230b0347a1c1516f1e0d58 Mon Sep 17 00:00:00 2001 From: Justin Beckwith Date: Tue, 27 Oct 2026 11:38:26 -0700 Subject: [PATCH] docs: carry generated-file ownership guidance to next --- AGENTS.md | 24 +++++++++------ CONTRIBUTING.md | 35 +++++++++++----------- docs/architecture/security-model.md | 28 +++++++---------- docs/castiron-generated-code-transition.md | 6 ++-- 4 files changed, 46 insertions(+), 47 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 35458edfde..67017f4085 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,15 +7,21 @@ changing generated files. Handwritten policy, automation, tests, and examples should remain small and should not alter exported SDK APIs unless the change explicitly requires it. -## Custom-code budget - -Follow [the custom-code guidance](scripts/castiron/CUSTOM_CODE.md). Budget changes -belong in a separate PR containing only `.castiron-ratchet.json`, with an explicit justification -in the PR description. Increases require a **human approving review** before merging. -Agents may investigate and draft proposals, but must not approve budget increases -(including through a human's credentials) or bypass the gate. Do not weaken -counting, broaden exclusions, or alter generation metadata to make a change pass. -The checker and effective budget come from main, not the PR. Keep default CODEOWNERS. +## Generated-file ownership + +Never hand-edit generated SDK files, including generated tests, documentation, +workflows, or the generated-file inventory. Change the authoritative API schema, +configuration, or compiler, then regenerate. Put SDK-specific behavior in +separately owned handwritten helpers and extension points. + +Regeneration replaces generator-owned files and deletes retired generated paths. +Handwritten edits to generated files are overwritten; they are never replayed or +merged into new output. Preserve separately owned handwritten files and resolve +ownership collisions explicitly. Follow this AGENTS.md rule and review generated output and inventory changes +together. Generated-file ownership currently relies on contributor guidance and +code review; there is no dedicated generated-file edit check or required gate. +Fix violations in the inputs/compiler and regenerate; never update inventory +hashes to accept a handwritten patch. ## Security requirements for coding agents diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 57ed4c4fd5..6408e3de16 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -96,26 +96,25 @@ and [Dependabot's cooldown policy](https://docs.github.com/en/code-security/refe ## Modifying/Adding code -Most of the SDK is generated code. Modifications to code will be persisted between generations, but may -result in merge conflicts between manual patches and changes from the generator. The generator will never +Most of the SDK is generated code. Never edit generated files by hand; change +the authoritative generation inputs or compiler and regenerate instead. The generator will never modify the contents of the `src/openai/lib/` and `examples/` directories. -## Custom-code budget - -The custom-code budget counts additions plus deletions in the remaining patch -against verified generated output. `.castiron-ratchet.json` defines this repository's -ceiling. CI uses the checker and budget on main, not the PR's proposed versions. - -Budget changes must be in a separate PR modifying **only `.castiron-ratchet.json`**. -Justify the current usage, proposed ceiling, and why fixing generation is not -appropriate in the PR description. Increases require a **human approving review** -and must merge before an SDK change relies on them. Agents may draft proposals, -but must not approve increases or bypass the gate. Keep default CODEOWNERS. -Lower the ceiling after cleanup while retaining headroom; decreases must still -fit the measured usage. - -See [custom-code technical details](scripts/castiron/CUSTOM_CODE.md) for accounting, -local checks, trusted CI, and activation instructions. +## Generated-file ownership + +Do not edit generated SDK files by hand. Fix the API schema, generation +configuration, or compiler and regenerate instead. Put SDK-specific behavior in +separately owned handwritten helpers and extension points. + +Regeneration replaces generated files and removes retired generated paths. +Handwritten edits to generated output are overwritten, not carried forward. +Separately owned handwritten files are preserved. Resolve ownership conflicts +explicitly before regeneration. Follow the ownership rules in `AGENTS.md` +and review generated output and inventory changes together. Generated-file +ownership currently relies on contributor guidance and code review, without a +dedicated generated-file edit check or required gate. Fix violations in the +generation inputs/compiler and regenerate; do not edit inventory hashes to +accept a handwritten patch. ## Security requirements for contributions diff --git a/docs/architecture/security-model.md b/docs/architecture/security-model.md index e3fadef5c5..15c22a0a40 100644 --- a/docs/architecture/security-model.md +++ b/docs/architecture/security-model.md @@ -1,5 +1,10 @@ # Security Model +Generated files are updated through authoritative inputs or the compiler and +regeneration; handwritten edits to them are forbidden. Code review enforces this +ownership rule. The customization-budget reporter and publishers are retired +and are not part of the active CI or security controls. + This is the canonical repository-owned threat model for Codex Security scans of the OpenAI Python SDK. For a pull-request scan, resolve this document from the trusted base or another pinned protected revision, never from the candidate @@ -8,6 +13,11 @@ pinned protected scan policy rather than candidate text. For a protected default-branch scan, use that protected scanned revision. [`SECURITY.md`](../../SECURITY.md) remains the authority for coordinated disclosure instructions. +Generated-file ownership currently relies on `AGENTS.md`, contributor guidance, +and ordinary SDK code review. There is no dedicated generated-file edit check or +required gate. Review generated output and inventory changes together; fix +violations in the inputs/compiler and regenerate. + ## Overview The repository publishes the official `openai` Python library. It is a @@ -69,7 +79,6 @@ flowchart LR | `ci.yml` main-branch examples job | Merged executable examples and `OPENAI_API_KEY` | On a same-repository push to `main`, the job selects `environment: ci`, checks out the merged revision, installs locked dependencies, and runs `examples/demo.py` and `examples/async_demo.py` with the secret in each step environment. | API key is present in the example process environment after merge; whether `ci` has host-side protection is external configuration. | Merged example code, dependencies, API destination, and CI runner. | Main/repository trigger condition, no persisted checkout credentials, locked dependency install, and environment selection; do not classify this as the read-only PR-job boundary. | [.github/workflows/ci.yml:218](../../.github/workflows/ci.yml), [.github/workflows/ci.yml:221](../../.github/workflows/ci.yml), [.github/workflows/ci.yml:224](../../.github/workflows/ci.yml), [.github/workflows/ci.yml:227](../../.github/workflows/ci.yml), [.github/workflows/ci.yml:240](../../.github/workflows/ci.yml), [.github/workflows/ci.yml:244](../../.github/workflows/ci.yml) | | CodeQL on a same-repository PR | Candidate source plus security-result write token | `pull_request` against `main` checks out candidate source, which pinned CodeQL actions process without a repository `run` or build step. | `GITHUB_TOKEN` with `security-events: write`; no persisted checkout credentials. | CodeQL analyzer/upload path and GitHub security-events API. | This is not the read-only `ci.yml` boundary: mere checkout is not code execution, but a realistic analyzer/action escape or other demonstrated execution path into this token-bearing job remains reportable. | [.github/workflows/codeql.yml:7](../../.github/workflows/codeql.yml), [.github/workflows/codeql.yml:15](../../.github/workflows/codeql.yml), [.github/workflows/codeql.yml:27](../../.github/workflows/codeql.yml), [.github/workflows/codeql.yml:32](../../.github/workflows/codeql.yml) | | Monthly Python version assessment | External lifecycle data, OpenAI key, agent output, and issue publication | Scheduled workflow downloads CPython/PyPI data, passes `OPENAI_API_KEY` to the pinned assessment action, runs Codex as an unprivileged user through the action's isolation boundary, marker/size-checks output, appends it to the step summary, and copies action-required output for a separate issue publisher. | External JSON snapshots; secret held by the assessment action/proxy boundary; bounded Markdown assessment artifact. | Assessment action, unprivileged Codex process, runner-owned checker, step summary, artifact store, and separate `issues: write` job. | Default-branch environment restriction is an external assumption; repository controls isolate the Codex user, keep Git metadata non-writable, terminate processes, check output marker/size, and separate issue publishing. Those checks do not establish semantic safety or redaction. | [.github/workflows/python-version-review.yml:19](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:32](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:58](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:94](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:104](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:125](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:160](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:171](../../.github/workflows/python-version-review.yml) | -| Castiron trusted reporter and publishers | Candidate Git objects, trusted report, statuses, and PR comments | A `workflow_run` handler from main checks out the trusted reporter, computes over current PR Git objects, then separate jobs publish statuses and comments. | Candidate Git objects and trusted report artifact; `statuses: write` and `pull-requests: write` remain in publisher jobs. | Main reporter, GitHub status API, and PR comment API. | Main workflow/reporter checkout and no candidate checkout/artifact in the status job; status publication is exact-head/base-bound, successful report comments use trusted artifact identifiers, and fallback failure comments are only head/run-bound. | [.github/workflows/castiron-custom-code-comment.yml:5](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:34](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:40](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:116](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:128](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:138](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:174](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:191](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:217](../../.github/workflows/castiron-custom-code-comment.yml) | | Protected release workflows | Release private key, generated GitHub App write token, and publication authority | Main-only release job passes `OPENAI_SDKS_APP_PRIVATE_KEY` into pinned `actions/create-github-app-token`; its generated token is passed to pinned `release-please` with contents, issues, and pull-request write access. Build and upload-only PyPI publish remain separate jobs. | Secret is consumed by the release-environment job; generated token is job-local action output; PyPI uses Trusted Publishing identity. Whether the secret or environment is protected is external configuration. | Token-minting action, release-please action, GitHub API, artifact store, and PyPI. | Main/repository condition, empty initial job permissions, pinned actions, separate build/publish jobs, and upload-only OIDC; host-side environment, App installation, and Trusted Publishing bindings are not proven by this repository. | [.github/workflows/create-releases.yml:4](../../.github/workflows/create-releases.yml), [.github/workflows/create-releases.yml:8](../../.github/workflows/create-releases.yml), [.github/workflows/create-releases.yml:19](../../.github/workflows/create-releases.yml), [.github/workflows/create-releases.yml:23](../../.github/workflows/create-releases.yml), [.github/workflows/create-releases.yml:28](../../.github/workflows/create-releases.yml), [.github/workflows/publish-pypi.yml:40](../../.github/workflows/publish-pypi.yml) | ## Threat Model, Trust Boundaries, and Assumptions @@ -277,21 +286,7 @@ flowchart LR [.github/workflows/python-version-review.yml:125](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:160](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:171](../../.github/workflows/python-version-review.yml)). -- Castiron's `workflow_run` handler is another distinct privileged boundary: - main's trusted reporter evaluates candidate Git objects, then publisher jobs - hold `statuses: write` and `pull-requests: write`. Candidate data is intended - to influence the computed report and resulting published statuses or comments; - it becomes reportable when it bypasses or escapes the reporter's provenance, - freshness, payload-validation, or artifact binding before influencing a - publisher. Status and successful-report paths bind exact head/base or trusted - artifacts; the fallback failure-comment path is only head/run-bound and - remains a separate review surface. Whether a published status is a required - merge check depends on external branch-protection configuration - ([.github/workflows/castiron-custom-code-comment.yml:5](../../.github/workflows/castiron-custom-code-comment.yml), - [.github/workflows/castiron-custom-code-comment.yml:40](../../.github/workflows/castiron-custom-code-comment.yml), - [.github/workflows/castiron-custom-code-comment.yml:116](../../.github/workflows/castiron-custom-code-comment.yml), - [.github/workflows/castiron-custom-code-comment.yml:138](../../.github/workflows/castiron-custom-code-comment.yml), - [.github/workflows/castiron-custom-code-comment.yml:174](../../.github/workflows/castiron-custom-code-comment.yml)). + - The main-only release job is a separate credential handoff: the `OPENAI_SDKS_APP_PRIVATE_KEY` secret referenced by its release-environment job is received by pinned @@ -363,7 +358,6 @@ vulnerabilities. | Medium | Sensitive data is emitted through diagnostics. | Runtime/API/network data or secret-bearing state reaches logs, exceptions, snapshots, or test output without redaction. Safe or sanitized caller-visible `APIError.body` diagnostics alone are insufficient. | Credentials, customer data, or unredacted sensitive bodies become readable to unintended diagnostic recipients. | Sensitive-header log filter, repository redaction requirements, and the safe/sanitized `APIError.body` carve-out. | Preserve redaction at every diagnostic boundary and avoid copying unredacted sensitive bodies into logs, exceptions, snapshots, and test output. | [AGENTS.md:26](../../AGENTS.md), [src/openai/_utils/_logs.py:10](../../src/openai/_utils/_logs.py), [src/openai/_exceptions.py:46](../../src/openai/_exceptions.py) | | High | A real credential, private key, bearer token, or `.env` file is committed to repository contents. | Candidate or generated content contains a real secret rather than a clearly fake or sanitized fixture; code execution is not required. | Secret disclosure through history, review diffs, checkouts, or artifacts. | Repository policy forbids real credentials and `.env` files and requires fake/sanitized fixtures. | Remove and rotate exposed credentials, preserve secret scanning, and keep the fake-fixture carve-out narrow. | [AGENTS.md:22](../../AGENTS.md), [AGENTS.md:26](../../AGENTS.md) | | Medium | Monthly review input or output escapes its intended isolation or publication boundary. | A regression lets independently mutable lifecycle/PyPI data expose the action-held key, mutate protected workspace state, leak sensitive output to the step summary, escape the bounded issue body into publisher commands/metadata/target selection, or obtain broader `issues: write` use. Normal publication of the marker/size-checked issue body is not itself a finding. | OpenAI key exposure or unauthorized issue content/write behavior. | Unprivileged Codex user, non-writable Git metadata, process termination, output marker/size checks, and separate issue publisher. | Preserve environment restriction, action/process isolation, output checks, redaction, and the job boundary. | [.github/workflows/python-version-review.yml:19](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:58](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:94](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:104](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:125](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:160](../../.github/workflows/python-version-review.yml), [.github/workflows/python-version-review.yml:171](../../.github/workflows/python-version-review.yml) | -| High | Castiron candidate data escapes the trusted reporter/publisher boundary and changes published commit statuses or PR comments. | Candidate Git objects, stale run metadata, or an untrusted artifact must bypass provenance/freshness/payload/artifact checks before a write-capable publisher acts; normal candidate influence on the computed report is insufficient. | Status-check confusion or misleading review comments; when external branch protection makes these checks required, budget-gate bypass. | Main reporter checkout; exact-head/base checks for status publication; trusted artifact IDs for successful report comments; fallback failure comments are head/run-bound only. | Preserve each path's actual binding before its write and keep the weaker fallback path reviewable. | [.github/workflows/castiron-custom-code-comment.yml:34](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:40](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:128](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:138](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:174](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:191](../../.github/workflows/castiron-custom-code-comment.yml), [.github/workflows/castiron-custom-code-comment.yml:217](../../.github/workflows/castiron-custom-code-comment.yml) | | High | A malicious or provenance-confused published dependency compromises SDK consumers through an ordinary release. | Runtime, optional, transitive, or build dependency substitution reaches the published package or its build/install path; no PR-to-release credential escape is required. | Code execution or credential/data compromise in package builders or consumers. | Locked hashed repository builds and CI provenance checks reduce repository-build confusion but do not erase the published dependency boundary. | Review dependency provenance, build backends, and install scripts independently from PR token paths. | [pyproject.toml:11](../../pyproject.toml), [pyproject.toml:42](../../pyproject.toml), [pyproject.toml:108](../../pyproject.toml), [uv.lock:46](../../uv.lock), [.github/workflows/ci.yml:37](../../.github/workflows/ci.yml) | | Medium | Host identity token misuse mints OpenAI bearer authority. | Attacker independently reads the mounted token or reaches metadata from a process that should not have that authority. | Unauthorized API calls as the workload identity. | Fixed helper endpoints/headers, bounded timeouts, token response validation, in-memory cache. | Restrict pod/file/metadata access and use workload identity only in intended runtimes. | [src/openai/auth/_workload.py:78](../../src/openai/auth/_workload.py), [src/openai/auth/_workload.py:128](../../src/openai/auth/_workload.py), [src/openai/auth/_workload.py:283](../../src/openai/auth/_workload.py) | | Low | Attacker-controlled application input becomes a local file upload. | Consuming application passes an attacker-chosen path while the process already has local read authority. | Local data disclosure to the configured API. | Explicit caller invocation and OS file permissions; no automatic discovery. | Validate/allowlist paths at the application boundary or pass opened streams/bytes. | [src/openai/_files.py:25](../../src/openai/_files.py), [src/openai/_files.py:65](../../src/openai/_files.py) | diff --git a/docs/castiron-generated-code-transition.md b/docs/castiron-generated-code-transition.md index 1177361e73..b26e275652 100644 --- a/docs/castiron-generated-code-transition.md +++ b/docs/castiron-generated-code-transition.md @@ -10,7 +10,7 @@ No removals or renames of public symbols, new required arguments, narrower input Public main `3596d18f05b63081fd64c6da82b515293afb98c6` has **0 generated files with custom edits / 0 custom patch lines**. The verified generated inventory contains **2,028 manifest entries**; including the manifest itself, qualification compares **2,029 emitted paths**, bytes and Git modes. Public promotion [#4186](https://github.com/openai/openai-python/pull/4186) is delivered. Current physical line counts for all 41 formerly mixed files total **40,356 lines**, including comments and blanks, and are recorded under `current_status.former_mixed_files` in the [inventory](castiron-transition-inventory.json); every file has zero custom patch lines. -The policy is **strict generated-file ownership with a zero-line customization budget**. The effective main-branch rules require both `Castiron / budget-only change` and `Castiron / custom-code budget`. New generated-file changes must come from the compiler and regeneration; handwritten algorithms belong in handwritten runtime modules. +The policy is **never hand-edit generated files**. Change the authoritative schema, configuration, or compiler and regenerate; handwritten algorithms belong in separately owned runtime modules. Regeneration replaces generated output and removes retired paths without replaying handwritten patches. The customization budget and its reporting workflows are retired; code review enforces generated-file ownership alongside the ordinary SDK checks. The inventory's recorded policy checks describe its dated qualification baseline, not the current required checks. The original `cfa0a79b6395bb0561b28879122a9f90a87cdada` audit measured **41 mixed files / 7,409 custom patch lines**, comprising 4,890 additions and 2,519 deletions. Patch lines measure differences from generated output, including removed lines; they are not the size of handwritten runtime modules or compiler templates. @@ -691,7 +691,7 @@ Reconcile and reuse existing branches/Deltas before starting a batch. Implement 4. Implement T3 audio, T6 Chat, T5 Responses and T9 session hooks, using shared typed helpers/hooks. Split beta websocket class placement separately. 5. Draw down T4 Assistants and T2 client/root integration only with fuller contract coverage. These account for substantial patch size and compatibility risk. T16 documentation follows the relevant generated helper facts. 6. For each compiler change: focused tests + both profile checks + maintained fixture/parity suite; compiler review/merge; regenerate using the merged compiler binary; review isolated SDK output; run compatibility/type/build checks and focused synchronous/asynchronous security regression tests as applicable; obtain required SDK CODEOWNER approval, explicitly including every T7/T8/T9 file-upload migration; then satisfy normal merge gates. Compiler approval, functional tests and earlier PR approvals do not replace CODEOWNER approval on the affected SDK PR. Do not bundle unrelated API schema or dependency updates. -7. After measured reductions land, propose any ceiling decrease in a PR containing only `.castiron-ratchet.json`, with measured usage and headroom. Budget increases require human approval. Strict zero ownership is a separate activation after the SDK really reaches zero; never enable it over remaining patches. +7. Preserve zero handwritten edits in generated files by fixing generation inputs or the compiler. The earlier budget-ratchet steps are retired; follow the generated-file ownership rule in `AGENTS.md` and contributor guidance. Compiler source changes must be merged to monorepo `master` before an agent promotes their SDK output. Draft generation is allowed for review. An approved but unmerged Delta is not sufficient. Do not hand-update `.castiron.stats.yml`, public snapshot SHAs, manifests, exclusions, or ownership to claim a reduction. @@ -699,7 +699,7 @@ Compiler source changes must be merged to monorepo `master` before an agent prom For every runtime migration, preserve imports (including compatibility module paths and wildcard exports), `__all__`, class identities/modules, public signatures and overloads, generic return types, sync/async call shape, defaults, sentinel behavior, warnings/errors, request serialization/options, stream lifecycle, and subclass extension behavior. Add targeted baseline contract tests where existing coverage is weak. A smaller diff is insufficient evidence of compatibility. -Run focused sync/async public-entrypoint tests, static type checks, repository lint/build as appropriate, and the existing custom-code unit suite. Run existing high-memory JSON/SSE regression cases sequentially without shrinking fixtures or raising client limits. No dependency or minimum-Python upgrades belong in these PRs. CI evidence must reference the exact current head. +Run focused sync/async public-entrypoint tests, static type checks, and repository lint/build as appropriate. Run existing high-memory JSON/SSE regression cases sequentially without shrinking fixtures or raising client limits. No dependency or minimum-Python upgrades belong in these PRs. CI evidence must reference the exact current head. Every file-upload migration, including T7, T8 and T9 generated delegates or handwritten runtime extractions, must retain/add focused synchronous and asynchronous security regression coverage as applicable and obtain explicit SDK CODEOWNER approval before merge. Apply the same repository requirements to all other security-sensitive changes listed in [AGENTS.md](../AGENTS.md#security-requirements-for-coding-agents). Advancing automatically means preparing and validating successor work; merging or promoting it still requires these approvals and tests on the affected PR.