Skip to content

fix(codex): restore Windows shim caller token state - #3956

Merged
lidge-jun merged 2 commits into
devfrom
codex/248-b-shim-8456
Sep 8, 2026
Merged

fix(codex): restore Windows shim caller token state#3956
lidge-jun merged 2 commits into
devfrom
codex/248-b-shim-8456

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Summary

Fix Windows Codex wrappers leaving a service-loaded token in the caller shell after Codex exits. CMD now scopes its environment; PowerShell restores the previous token presence/value in a finally block while preserving the Codex exit status. The change affects newly generated wrappers, and the five existing lifecycle guides explain how to recreate an older wrapper.

Carries #3909 from 06829fa566c5e9d688a934fd31d481227450828b onto current dev, preserving luvs01's original production patch. Regression coverage additionally separates explicit-token execution from bypass and checks exact caller state and completed nonzero failures after PowerShell exceptions. This PR contains only the Windows shim bug; other B-workstream fixes have independent PRs.

Verification

  • Local runtime tests and suites: NOT RUN per owner instruction. No local validation is claimed.
  • During initial baseline synchronization, an existing post-merge hook unexpectedly ran dependency installation, type checking and a build. This was disclosed to the owner and excluded from verification evidence. Subsequent Git mutations disable hooks per invocation; pushes use --no-verify.
  • git diff --check passed. Production src/codex/shim.ts is byte-identical to the original source PR head. The newer dev documentation remains intact.
  • Independent source-plan audit passed. Final independent source/security review passed on 5643743439ef4fe4ee7039d45f6bef4b5d88723d: no blocking findings; seven files covered. This is source review, not runtime or human review approval.
  • Current-head PR CI and a separate ci.yml workflow dispatch with lane=all are required before landing. Ordinary PR CI skips Windows shards and macOS control; these skips are not passing Windows evidence.
  • Tests use isolated homes, synthetic token values and fake Codex/ensure programs. No real credentials, service operations or account credits are used.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Co-authored-by: luvs01 27862058+luvs01@users.noreply.github.com

Summary by CodeRabbit

  • New Features

    • Windows CMD and PowerShell shims now preserve the caller’s OPENCODEX_API_AUTH_TOKEN after execution.
    • Added guidance for recreating existing Windows shims to apply this behavior.
  • Documentation

    • Documented Windows token-environment behavior in the CLI lifecycle reference across supported languages.
  • Bug Fixes

    • Preserved correct command exit statuses when restoring the token environment.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: 3807c08b-ff80-49c0-93e0-a9e112f0071b

📥 Commits

Reviewing files that changed from the base of the PR and between bb1b798 and d6f9eca.

📒 Files selected for processing (7)
  • docs-site/src/content/docs/ja/reference/cli/lifecycle.md
  • docs-site/src/content/docs/ko/reference/cli/lifecycle.md
  • docs-site/src/content/docs/reference/cli/lifecycle.md
  • docs-site/src/content/docs/ru/reference/cli/lifecycle.md
  • docs-site/src/content/docs/zh-cn/reference/cli/lifecycle.md
  • src/codex/shim.ts
  • tests/codex-integration/codex-shim.test.ts

📝 Walkthrough

Walkthrough

Windows CMD and PowerShell shims now restore the caller’s OPENCODEX_API_AUTH_TOKEN. Integration tests cover success and failure paths. CLI lifecycle documentation describes the behavior and recreation steps for existing Windows shims.

Changes

Windows shim token restoration

Layer / File(s) Summary
Shim environment restoration
src/codex/shim.ts
CMD shims use setlocal. PowerShell shims restore the previous token state and preserve the Codex exit code.
Windows integration coverage
tests/codex-integration/codex-shim.test.ts
Windows tests cover multiple shells, caller token states, ensure and bypass modes, successful execution, and failure handling.
CLI lifecycle documentation
docs-site/src/content/docs/.../reference/cli/lifecycle.md
Japanese, Korean, Russian, Simplified Chinese, and English references document token restoration and the uninstall/reinstall procedure for existing shims.

Estimated code review effort: 3 (Moderate) | ~20 minutes

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/248-b-shim-8456

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lidge-jun lidge-jun added the maintainer-sponsored Maintainer sponsors this change to an auth, workflow, release, or dependency surface label Sep 7, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

Explicit security review record for head 5643743439ef4fe4ee7039d45f6bef4b5d88723d, base 514350e6f79ed4539378388bc39d3fc79ff2c70c.

The owner authorized agent-assisted maintainer delivery of this scoped change. Independent Astra high source/security review and main-agent source verification found no blocking issue. Production shim bytes match the contributor head. We reviewed token presence/value restoration, nested CMD scope and batch handoff, PowerShell finally and exit status, escaped fixture paths, no-timeout failure oracles, and preservation of existing-installation ownership. Codex and descendants intentionally remain token recipients. No real account or token was used.

This records the explicit security review and sponsorship decision for this revision. It is not a claim of a separate human approval or passing Windows execution. Required PR checks and exact-head workflow_dispatch lane=all (six Windows shards plus macOS control) remain merge gates. Any source change reopens this review.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 7, 2026
@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 66 / 80

이 PR은 Windows Codex shim이 끝난 뒤 호출자 셸에 서비스에서 읽은 OPENCODEX_API_AUTH_TOKEN을 남기는 문제를 고칩니다. 원본은 @luvs01의 #3909이고, 커밋 06829fa56을 현재 dev 위로 캐리했습니다. 지금 HEAD의 src/codex/shim.ts를 보면 CMD 래퍼는 토큰 파일을 읽은 뒤 바깥 setlocal 없이 호출자 환경에 쓰고, PowerShell 래퍼는 ensure 런타임 env만 finally로 되돌리고 토큰은 되돌리지 않습니다. Codex와 자식은 토큰을 상속할 수 있어도, 호출자 셸에 서비스 토큰이 남는 것은 원치 않는 노출입니다.

고치는 방법은 단순합니다. CMD는 스크립트 맨 위에 setlocal을 넣어 배치가 끝날 때 호출자 env가 원래대로 돌아가게 합니다(ensure 구간의 안쪽 setlocal/endlocal은 그대로). PowerShell은 시작 시 토큰 존재 여부와 값을 저장한 뒤 try/finally에서 복원하고, Codex exit code는 $codexExitCode로 보존한 채 exit합니다. 새로 생성되는 래퍼에만 적용되므로, 이미 깔린 Windows shim은 ocx codex-shim uninstallinstall로 다시 만들어야 합니다. 그 안내가 en/ko/ja/zh-cn/ru lifecycle 문서에 같은 내용으로 들어갔습니다.

회귀 테스트는 tests/codex-integration/codex-shim.test.ts에 Windows 전용(skipIf not win32)으로, cmd/powershell/pwsh × missing/empty/explicit × ensure/bypass, 그리고 ensure/Codex 예외 후에도 호출자 상태가 복구되는지까지 있습니다. 라벨은 bug + maintainer-sponsored이고 지금은 draft입니다. types/config 분할과 무관합니다. #3909는 아직 열려 있으니 이 캐리가 랜딩 차량입니다.

라인 src/codex/shim.ts · buildWindowsCodexShim - 맨 위 setlocal은 맞습니다. 다만 ensure 구간에 이미 안쪽 setlocal/endlocal이 있어서, 리뷰어가 “중복인가?”라고 물을 수 있습니다. 역할이 다릅니다. 바깥은 호출자 토큰 격리, 안쪽은 ensure용 bun runtime env 격리입니다. 주석 한 줄이 있으면 더 읽기 쉽습니다.

라인 src/codex/shim.ts · buildWindowsPowerShellCodexShim - finally에서 토큰을 지우고 exit code를 보존하는 순서가 맞습니다. $codexExitCodetry 안에서 잡고 finallyexit하는 형태라, ensure/Codex가 throw 해도 호출자 복구는 됩니다.

경로 docs-site/.../lifecycle.md (5 locale) - 기존 shim은 자동으로 안 바뀐다는 점이 명확합니다. 업데이트만으로는 안 고쳐지니, 릴리스 노트나 트러블슈팅에 한 번 더 짚을지는 선택입니다.

경로 tests/codex-integration/codex-shim.test.ts - 의미 있는 매트릭스입니다. Linux/macOS 호스트에서는 이 케이스가 전부 skip되므로, Windows CI 샤드가 이 head에서 실제로 도는지가 머지 게이트입니다.

메인테이너의 판단이 필요한 지점

  • draft를 언제 undraft/merge할지 (Windows 테스트 샤드 그린 확인 후인지)
  • 이미 설치된 shim 사용자에게 uninstall/install을 릴리스 노트에 넣을지, lifecycle note만으로 충분한지
  • 머지 후 #3909에 Landed via #3956 + landed-via-maintainer close를 같은 턴에 할지

너의 추천
Windows CI가 그린이면 undraft 후 머지하세요. 호출자 셸에 서비스 토큰이 남는 문제는 실제 노출 버그입니다. 머지 후 #3909는 leftover로 닫고, 원본을 리베이스하지 마세요. 기존 설치본 안내는 lifecycle note로 충분해 보이지만, 안정 릴리스 노트에 한 줄 넣으면 더 안전합니다.

이 댓글은 grok-bot이 작성했습니다

luvs01 and others added 2 commits September 8, 2026 10:58
…ation

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@lidge-jun
lidge-jun force-pushed the codex/248-b-shim-8456 branch from 5643743 to d6f9eca Compare September 8, 2026 01:58
@lidge-jun

Copy link
Copy Markdown
Owner Author

Refreshed candidate d6f9eca6999500b953de82afd0710f16c8941d0a on bb1b79859d472ef292a6f812960122a0389c8f2c includes independently landed routing/probe fixture fixes. Both own commits are unchanged by range-diff; all seven patch files match the prior reviewed candidate. Fresh independent source/security interdiff PASS. New PRCI34178467225 and full dispatch34178464441 pending; previous failure retained and not counted as green. No local product execution.

@lidge-jun
lidge-jun marked this pull request as ready for review September 8, 2026 02:28
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 8, 2026 02:28
@lidge-jun

Copy link
Copy Markdown
Owner Author

Owner-authorized dev-only maintainer integration for d6f9eca into 2ee9019. Independent source review PASS; exact-head hosted verification: 34178467225 pull_request: 19 success / 2 skip; 34178464441 workflow_dispatch: 26 success / 0 skip. Skips are not passing tests. Original failure history retained; no local product tests/build/typecheck/install. One fixture repair only, independently revertible squash. Independently reviewed fixture/voice changes in destination: tests/responses/responses-state.test.ts. Candidate-scoped evidence reused; whole combined-tree execution is not claimed. Remaining B candidate CI will verify the cumulative baseline. Actual landing is checked against precomputed merge tree, unchanged unrelated paths and fetched dev ancestry. This is agent-assisted maintainer integration, not a separate human approval. No unresolved threads or maintainer objections; ready-triggered optional review checks are not claimed as passed.

@lidge-jun
lidge-jun merged commit 4d46290 into dev Sep 8, 2026
55 of 56 checks passed
@lidge-jun
lidge-jun deleted the codex/248-b-shim-8456 branch September 8, 2026 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working maintainer-sponsored Maintainer sponsors this change to an auth, workflow, release, or dependency surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants