Harden patch_chatgpt_providers.py - #1
Closed
ydeng11 wants to merge 3 commits into
Closed
Conversation
- Fix sudo config ownership (SUDO_USER policy, chown failures raise PatchError) - Robust staging-sibling restore with combined diagnostics - README verification: hash vs plist, mixed markers, picker symbol, signature - Strict JS-aligned config validation (keep/create/replace states) - Defer process shutdown and config commit until app verified - Derive target executable from CFBundleExecutable - Add --self-test mode (24 tests) + PLAN.md
ydeng11
changed the title
The 26.727 rename tables mapped the injected loaders' host-file requests from tp() to rp(), but rp() only dispatches the app-server request map (model/list, thread/skill requests). The real bundle calls codex-home and read-file via the local-host bridge bp() (vscode://codex/<method>), so the config always failed to load and the picker fell back to the embedded OpenRouter config. Revert those two renames to bp(); verified in the installed bundle (bp(codex-home) x5, bp(read-file) x7, rp(...) 0) and all 28 self-tests pass.
The upgraded 26.727.51351 / build 6119 bundle renamed the app-server params getter (rSe -> nSe) and rewrote the model-picker menu (the item component now renders the config's extras slot natively, so the separate item-component patch is unnecessary). Adds: - CENTRAL_DIFF_6119: 26.727 central diff with the nSe rename. - PICKER_DIFF_6119: injection + model-list prepend + React binding adapted to the rewritten picker, symbol-renamed (CZ/SZ, _z/mz). - PATCH_VARIANTS entry so the installer matches build 6119 bundles. Verified: 28 self-tests pass, variant applies uniquely to the real 6119 bundle (prettier + node clean), install verified via the READY path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardens
patch_chatgpt_providers.py(the macOS installer that injects the custom model-provider picker + per-model provider routing into the desktop Codex client) based on an advisor review. All six findings are fixed; the version-sensitive exact-hunk design and every embeddedCENTRAL_DIFF*/PICKER_DIFF*hunk are untouched (byte-identical).Changes
Sudo config ownership (blocker) — When run via
sudo, the default~/.codex/desktop-model-providers.jsonis now owned by the invoking user (file + freshly created parent dirs). Custom--configpaths are never reassigned toSUDO_USER: existing files preserve their owner, missing files stay root. Requiredchownfailures raisePatchErrorinstead of being swallowed.Robust restore —
restore_backup()restores into a sibling staging directory, verifies it, then atomically swaps. Any failure raises a combined diagnostic namingbackup,failed_copy, andstaging(including swap, move-back, and cleanup error details) and never silently leaves the app path absent.Honest READY detection — "Already installed" is only reported after
verify_installed_bundle()passes: ASAR header hash vsInfo.plist, V3 marker present, V2 marker absent (mixed markers rejected), picker symbol present, andcodesign --verify --deep --strictsucceeds (stderr preserved in the error). The fresh-install path uses the same unified check.Strict, JS-aligned config validation —
versionmust be the integer1(bools/1.0/"1"/Nonerejected), string checks precede set membership (no unhashableTypeError), and mapping values are matched untrimmed to mirrorcodexNormalizeProviderRoutingConfig. Config state is modeled askeep/create/replace: onlykeepvalidates the existing file, so--overwrite-configrepairs malformed JSON instead of rejecting it.Deferred mutations — Target-app processes are stopped only after the patched asar is packed and verified (just before backup); the provider-routing config is committed only after codesign + final ASAR checks, so an unsupported or failed install never disturbs a running app or clobbers user configuration.
Executable from
CFBundleExecutable—stop_target_app_processes()no longer hardcodesContents/MacOS/ChatGPT; it derives the name fromInfo.plist.--self-testmode — 28 unit tests (no app/network access): config validation/lifecycle, diff mechanics (render_unified_diff,parse_hunks,derive_versioned_diff,unique_candidate,contains_markerchunk boundaries), synthetic per-variant fixtures proving unique layout matching + repeat-application rejection, fake-ASAR READY-state rejection (hash mismatch / mixed markers / missing picker), codesign-failure conversion, restore diagnostics, and ownership-policy tests with mocked root/SUDO_USER.Verification
python3 -m py_compile patch_chatgpt_providers.py— cleanpython3 patch_chatgpt_providers.py --self-test— all 28 tests pass, exit 0python3 patch_chatgpt_providers.py --help— renders, lists--self-testPLAN.mdis included as the plan/design record for the change.