fix: publish Castiron checks for external-fork pull requests - #2464
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Castiron custom code✅ No new custom-code files detected. 32 mixed files remain; 2 existing customizations changed. Compared
30 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 32879001656 --repo openai/openai-node \
--name castiron-custom-code-32879001656-1 --dir /tmp/castiron-custom-code-32879001656-1
git apply --stat /tmp/castiron-custom-code-32879001656-1/custom-code.patch
cat /tmp/castiron-custom-code-32879001656-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin c100c832962ec42e7777ba506bba9b34e3520d52 ee9ee16e1715f41436bfef6f5954d353f9e9a064
python3 scripts/castiron/custom_code_report.py report \
--base c100c832962ec42e7777ba506bba9b34e3520d52 \
--head ee9ee16e1715f41436bfef6f5954d353f9e9a064 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-ee9ee16e1715
cat /tmp/castiron-custom-code-ee9ee16e1715/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed the fork workflow-run association and privileged Castiron reporting paths, including independent upstream PR/head/source-repository/base validation, stale-run handling, artifact isolation, least-privilege status/comment publication, and the expanded security regression coverage. LGTM.
apcha-oai
left a comment
apcha-oai
left a comment
There was a problem hiding this comment.
[sdk codex review] Approving ee9ee16e1715f41436bfef6f5954d353f9e9a064 after two independent full reviews of all six changed files and their reporting/publication contracts. No substantiated blocking issue found.
Fork-side associations remain discovery hints: the resolver and both privileged publishers re-fetch upstream PRs and validate source repository, exact head, open state, intended base and a single current association. Trusted computation/artifact boundaries, least-privilege permissions, stale-run handling and merge-group checks remain intact. The new regressions exercise the actual embedded publisher JavaScript; no duplicate review comments are needed.
Primary Castiron CI checked out this exact head, verified the reporter checksum, and ran 60 focused tests successfully with one skip. Node CI passed the Node 22/24/26 test matrix at the head, plus lint/type checks. OkTest passed 237 tests across 42 suites on merge 8c497a860f716e4e7111d64fcc711ca1b66b0a2f (head + base c100c832). Subsequent observed main changes affect only pnpm-lock.yaml, with no overlap with these workflow/Python paths; these results are not execution against that newer main.
Head, PR base, live base, author, open/non-draft status and mergeability rechecked before submission. Validation used source review, existing hosted logs and read-only GitHub API observations—not new scans, candidate execution or a deployed privileged fork-publisher test.
Raise the custom-code budget from 3,000 to 4,000 to unblock SDK generation. Main currently uses 2,983 lines; the current generation candidate uses 3,497, leaving 503 lines under the new limit. These remaining customizations are technical debt. We do not have time to address that debt at the moment, so this raises the budget to unblock the work and defers the underlying cleanup. Usage was 1,770 when the original budget landed in openai#2446. The increase on main came from: - Reporter and test changes: +696 lines in openai#2447 and openai#2464. - X.509 client integration: +513 lines in openai#2472, openai#2473 and openai#2479. - Webhook verification wiring: +4 lines in openai#2475. Only `.castiron-ratchet.json` changes.
3 participants
Summary
GitHub omits
workflow_run.pull_requestsfor external-fork runs, and querying the upstream repository's commit-association endpoint returns no pull requests for those fork commits. That leaves the requiredCastiron / budget-only changeandCastiron / custom-code budgetcontexts permanently expected even when the candidate workflow succeeds.custom_code_report.pyand reuse it from trusted report generation, comment publication, and budget evaluation.head_repository, including legitimately renamed forks, then independently re-fetch every candidate PR fromopenai/openai-node.Security model
Fork-side associations and PR numbers are discovery hints, never authorization. The trusted paths:
head_repositorymetadata; reject traversal-like components and spoofed identities.mainbase ref, and exactly one valid current association.mainbase SHA wherever budget evaluation or status publication needs freshness; preserve existing stale-run behavior and merge-group validation.The existing trusted
workflow_run/main-checkout boundary, bare Git object store, candidate-artifact isolation, least-privilege job permissions, merge-queue protections, and exact required status names remain unchanged. No candidate workflow definition, mutable ref, contributor artifact, or fork-supplied PR number is trusted.Affected contributor PRs
32877584723haspull_requests: []; the updated resolver correctly finds its upstream PR throughcmun2/openai-node.Verification
env PYTHONDONTWRITEBYTECODE=1 python3 -m unittest discover -s scripts/castiron -p 'test_custom_code*.py'— 60 tests pass, with one pre-existing skip.go run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/castiron-custom-code.yml .github/workflows/castiron-custom-code-comment.yml— both workflows pass actionlint v1.7.12.ruff format --check scripts/castiron/custom_code_report.py scripts/castiron/custom_code_budget.py scripts/castiron/test_custom_code_report.py scripts/castiron/test_custom_code_budget.py.git diff --checkpasses.