Skip to content

fix: pre-main hardening logic must tolerate non-UTF-8 env vars #7749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
bolinfest merged 1 commit into from
Dec 9, 2025
Merged

fix: pre-main hardening logic must tolerate non-UTF-8 env vars #7749

bolinfest merged 1 commit into from
Dec 9, 2025

Conversation

@bolinfest
Copy link
Collaborator

@bolinfest bolinfest commented Dec 8, 2025 •
edited
Loading

We received a bug report that Codex CLI crashes when an env var contains a non-ASCII character, or more specifically, cannot be decoded as UTF-8:

$ RUST_BACKTRACE=full RÖDBURK=1 codex

thread '<unnamed>' panicked at library/std/src/env.rs:162:57:
called `Result::unwrap()` on an `Err` value: "RÃ\xB6DBURK"
stack backtrace:
   0:        0x101905c18 - __mh_execute_header
   1:        0x1012bd76c - __mh_execute_header
   2:        0x1019050e4 - __mh_execute_header
   3:        0x101905ad8 - __mh_execute_header
   4:        0x101905874 - __mh_execute_header
   5:        0x101904f38 - __mh_execute_header
   6:        0x1019347bc - __mh_execute_header
   7:        0x10193472c - __mh_execute_header
   8:        0x101937884 - __mh_execute_header
   9:        0x101b3bcd0 - __mh_execute_header
  10:        0x101b3c0bc - __mh_execute_header
  11:        0x101927a20 - __mh_execute_header
  12:        0x1005c58d8 - __mh_execute_header

thread '<unnamed>' panicked at library/core/src/panicking.rs:225:5:
panic in a function that cannot unwind
stack backtrace:
   0:        0x101905c18 - __mh_execute_header
   1:        0x1012bd76c - __mh_execute_header
   2:        0x1019050e4 - __mh_execute_header
   3:        0x101905ad8 - __mh_execute_header
   4:        0x101905874 - __mh_execute_header
   5:        0x101904f38 - __mh_execute_header
   6:        0x101934794 - __mh_execute_header
   7:        0x10193472c - __mh_execute_header
   8:        0x101937884 - __mh_execute_header
   9:        0x101b3c144 - __mh_execute_header
  10:        0x101b3c1a0 - __mh_execute_header
  11:        0x101b3c158 - __mh_execute_header
  12:        0x1005c5ef8 - __mh_execute_header
thread caused non-unwinding panic. aborting.

I discovered I could reproduce this on a release build, but not a dev build, so between that and the unhelpful stack trace, my mind went to the pre-main() logic we run in prod builds. Sure enough, we were operating on std::env::vars() instead of std::env::vars_os(), which is why the non-UTF-8 environment variable was causing an issue.

This PR updates the logic to use std::env::vars_os() and adds a unit test.

And to be extra sure, I also verified the fix works with a local release build:

$ cargo build --bin codex --release
$ RÖDBURK=1 ./target/release/codex --version
codex-cli 0.0.0

@bolinfest bolinfest changed the title fix: pre-main hardening logic must tolerate non-UTF8 env vars fix: pre-main hardening logic must tolerate non-UTF-8 env vars Dec 8, 2025
aibrahim-oai
aibrahim-oai approved these changes Dec 8, 2025
View reviewed changes
Copy link
Collaborator

@aibrahim-oai aibrahim-oai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!!

@bolinfest bolinfest merged commit 06704b1 into main Dec 9, 2025
71 of 73 checks passed
@bolinfest bolinfest deleted the pr7749 branch December 9, 2025 00:00
@github-actions github-actions bot locked and limited conversation to collaborators Dec 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@aibrahim-oai aibrahim-oai aibrahim-oai approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bolinfest @aibrahim-oai