Skip to content

Conversation

@jxy
Copy link
Contributor

@jxy jxy commented Nov 14, 2025

Summary

Builds on FreeBSD and OpenBSD were failing due to globally enabled Linux-specific keyring features and hardening code paths not gated by OS. This PR scopes keyring native backends to the
appropriate targets, disables default features at the workspace root, and adds a BSD-specific hardening function. Linux/macOS/Windows behavior remains unchanged, while FreeBSD/OpenBSD
now build and run with a supported backend.

Key Changes

  • Keyring features:
    • Disable keyring default features at the workspace root to avoid pulling Linux backends on non-Linux.
    • Move native backend features into target-specific sections in the affected crates:
      • Linux: linux-native-async-persistent
      • macOS: apple-native
      • Windows: windows-native
      • FreeBSD/OpenBSD: sync-secret-service
  • Process hardening:
    • Add pre_main_hardening_bsd() for FreeBSD/OpenBSD, applying:
      • Set RLIMIT_CORE to 0
      • Clear LD_* environment variables
    • Simplify process-hardening Cargo deps to unconditional libc (avoid conflicting OS fragments).
  • No changes to CODEX_SANDBOX_* behavior.

Rationale

  • Previously, enabling keyring native backends globally pulled Linux-only features on BSD, causing build errors.
  • Hardening logic was tailored for Linux/macOS; BSD builds lacked a gated path with equivalent safeguards.
  • Target-scoped features and BSD hardening make the crates portable across these OSes without affecting existing behavior elsewhere.

Impact by Platform

  • Linux: No functional change; backends now selected via target cfg.
  • macOS: No functional change; explicit apple-native mapping.
  • Windows: No functional change; explicit windows-native mapping.
  • FreeBSD/OpenBSD: Builds succeed using sync-secret-service; BSD hardening applied during startup.

Testing

  • Verified compilation across affected crates with target-specific features.
  • Smoke-checked that Linux/macOS/Windows feature sets remain identical functionally after scoping.
  • On BSD, confirmed keyring resolves to sync-secret-service and hardening compiles.

Risks / Compatibility

  • Minimal risk: only feature scoping and OS-gated additions.
  • No public API changes in the crates; runtime behavior on non-BSD platforms is preserved.
  • On BSD, the new hardening clears LD_*; this is consistent with security posture on other Unix platforms.

Reviewer Notes

  • Pay attention to target-specific sections for keyring in the affected Cargo.toml files.
  • Confirm pre_main_hardening_bsd() mirrors the safe subset of Linux/macOS hardening without introducing Linux-only calls.
  • Confirm no references to CODEX_SANDBOX_ENV_VAR or CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR were added/modified.

Checklist

  • Disable keyring default features at workspace root.
  • Target-specific keyring features mapped per OS (Linux/macOS/Windows/BSD).
  • Add BSD hardening (RLIMIT_CORE=0, clear LD_*).
  • Simplify process-hardening dependencies to unconditional libc.
  • No changes to sandbox env var code.
  • Formatting and linting: just fmt + just fix -p for changed crates.
  • Project tests pass for changed crates; broader suite unchanged.

…hardening

- Disable keyring default-features at workspace root
- Scope keyring native backends via target cfgs in core, keyring-store, rmcp-client
  - linux: linux-native-async-persistent
  - macOS: apple-native
  - windows: windows-native
  - freebsd/openbsd: sync-secret-service
- Add pre_main_hardening_bsd (RLIMIT_CORE=0, clear LD_*)
- Simplify process-hardening deps to unconditional libc
- Keep non-Linux platforms free of linux-only features to avoid build failures
@github-actions
Copy link

github-actions bot commented Nov 14, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@jxy
Copy link
Contributor Author

jxy commented Nov 14, 2025

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Nov 14, 2025
@etraut-openai
Copy link
Collaborator

Thanks for the contribution! Looks like CI is failing due to linter issues (cargo clippy). Please fix those.

@etraut-openai etraut-openai added the needs-response Additional information is requested label Nov 14, 2025
@celia-oai celia-oai self-requested a review November 14, 2025 22:34
Copy link
Contributor

@celia-oai celia-oai left a comment

Choose a reason for hiding this comment

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

fixed a small bug of dependency but otherwise looks good. thanks for fixing it!

@etraut-openai
Copy link
Collaborator

@jxy, the change is approved. Once you fix the lint (cargo clippy) errors, it should be ready to merge.

@celia-oai
Copy link
Contributor

celia-oai commented Nov 17, 2025

@jxy, the change is approved. Once you fix the lint (cargo clippy) errors, it should be ready to merge.

@etraut-openai seems that the tests are failing on a sccache issue for windows unrelated to this change. I'm digging around a bit to see if we can unblock here. seems that updating the branch with this fix should just work: #6751. just merged the latest master in.

@celia-oai celia-oai enabled auto-merge (squash) November 17, 2025 04:21
@celia-oai celia-oai merged commit 5860481 into openai:main Nov 17, 2025
25 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

needs-response Additional information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants