发布

  • feat: introduce support for shell_environment_policy in config.toml (#1061)

    frostbyte_neo 发布于 2025-05-22 16:51:19 +00:00

    To date, when handling shell and local_shell tool calls, we were
    spawning new processes using the environment inherited from the Codex
    process itself. This means that the sensitive OPENAI_API_KEY that
    Codex needs to talk to OpenAI models was made available to everything
    run by shell and local_shell. While there are cases where that might
    be useful, it does not seem like a good default.

    This PR introduces a complex shell_environment_policy config option to
    control the env used with these tool calls. It is inevitably a bit
    complex so that it is possible to override individual components of the
    policy so without having to restate the entire thing.

    Details are in the updated README.md in this PR, but here is the
    relevant bit that explains the individual fields of
    shell_environment_policy:

    | Field | Type | Default | Description |
    | ------------------------- | -------------------------- | ------- |

    |
    | inherit | string | core | Starting template for the
    environment:
    core (HOME, PATH, USER, …), all (clone full
    parent env), or none (start empty). |
    | ignore_default_excludes | boolean | false | When false, Codex
    removes any var whose name contains KEY, SECRET, or TOKEN
    (case-insensitive) before other rules run. |
    | exclude | array<string> | [] | Case-insensitive glob
    patterns to drop after the default filter.
    Examples: "AWS_*",
    "AZURE_*". |
    | set | table<string,string> | {} | Explicit key/value
    overrides or additions – always win over inherited values. |
    | include_only | array<string> | [] | If non-empty, a
    whitelist of patterns; only variables that match one pattern survive
    the final step. (Generally used with inherit = "all".) |

    In particular, note that the default is inherit = "core", so:

    • if you have extra env variables that you want to inherit from the
      parent process, use inherit = "all" and then specify include_only
    • if you have extra env variables where you want to hardcode the values,
      the default inherit = "core" will work fine, but then you need to
      specify set

    This configuration is not battle-tested, so we will probably still have
    to play with it a bit. core/src/exec_env.rs has the critical business
    logic as well as unit tests.

    Though if nothing else, previous to this change:

    $ cargo run --bin codex -- debug seatbelt -- printenv OPENAI_API_KEY
    # ...prints OPENAI_API_KEY...
    

    But after this change it does not print anything (as desired).

    One final thing to call out about this PR is that the
    configure_command! macro we use in core/src/exec.rs has to do some
    complex logic with respect to how it builds up the env for the process
    being spawned under Landlock/seccomp. Specifically, doing
    cmd.env_clear() followed by cmd.envs(&$env_map) (which is arguably
    the most intuitive way to do it) caused the Landlock unit tests to fail
    because the processes spawned by the unit tests started failing in
    unexpected ways! If we forgo env_clear() in favor of updating env vars
    one at a time, the tests still pass. The comment in the code talks about
    this a bit, and while I would like to investigate this more, I need to
    move on for the moment, but I do plan to come back to it to fully
    understand what is going on. For example, this suggests that we might
    not be able to spawn a C program that calls env_clear(), which would
    be...weird. We may still have to fiddle with our Landlock config if that
    is the case.

    下载附件