Skip to content

Fix Codex app-server permission approval handling - #27746

Open
scottjones08 wants to merge 3 commits into
NousResearch:mainfrom
scottjones08:fix/codex-app-server-permissions-20260518-001521
Open

scottjones08 wants to merge 3 commits into
NousResearch:mainfrom
scottjones08:fix/codex-app-server-permissions-20260518-001521

Conversation

@scottjones08

Copy link
Copy Markdown

Summary\n- route Codex app-server permission escalation requests through Hermes approvals instead of declining them unconditionally\n- safely auto-accept non-interactive permission grants only for configured writable roots/current cwd\n- update config UI schema/docs for the codex_app_server runtime and writable_roots setup\n\n## Verification\n- python3.11 -m py_compile agent/transports/codex_app_server_session.py hermes_cli/web_server.py\n- verified codex app-server can write into configured sibling repo /Users/scottjones/Documents/GitHub/mudanza-azure after restart\n

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/openai OpenAI / Codex Responses API labels May 18, 2026
@teknium1

teknium1 commented Jun 13, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks for the focused fix — the premise still reproduces on current main: agent/transports/codex_app_server_session.py:663 handles item/permissions/requestApproval by always responding {"decision": "decline"}.

Problems

  • The proposed writable-root helper reads only CODEX_HOME / ~/.codex per the PR diff, but CodexAppServerSession already accepts an explicit codex_home and passes it to the client (agent/transports/codex_app_server_session.py:205, agent/transports/codex_app_server_session.py:247). Sessions using that override would check the wrong Codex config.
  • This touches a security-sensitive approval path but adds no tests. Current tests cover exec/file-change approvals around tests/agent/transports/test_codex_app_server_session.py:451, and git grep -n "permissions/requestApproval" origin/main -- tests/agent/transports/test_codex_app_server_session.py tests/agent/transports/test_codex_app_server_runtime.py returns no coverage.

Suggested changes

  • Thread self._codex_home into writable-root parsing, falling back to env/home only when no explicit Codex home is configured.
  • Add tests for callback routing, callback failure fail-closed behavior, cwd/configured-root auto-accept, and outside-root decline.

Automated hermes-sweeper review; a human maintainer will make the final call.

@alt-glitch alt-glitch added comp/dashboard Web dashboard / control panel UI (dashboard/, landing) sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jun 26, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the focused Codex permission-escalation fix. The current main path still unconditionally declines item/permissions/requestApproval at agent/transports/codex_app_server_session.py:827-832, so the core premise remains valid.

Problems

  • PR head agent/transports/codex_app_server_session.py:128 reads writable roots only from CODEX_HOME or ~/.codex. The session already accepts codex_home and forwards self._codex_home to its client on current main (agent/transports/codex_app_server_session.py:206-215,247-249), so an explicit override can make the approval check inspect a different config from the app-server.
  • The new permission-decision path at PR head agent/transports/codex_app_server_session.py:655-657 has no tests. Existing bridge tests cover command and file-change requests at tests/agent/transports/test_codex_app_server_session.py:570-713, but not permissions/requestApproval.
  • The approvals.mode dashboard option correction is already on main in da6d6164b; it should be omitted during salvage.

Suggested changes

  • Thread self._codex_home into writable-root parsing, then fall back to CODEX_HOME/home only when unset.
  • Add callback, callback-failure, allowed-root, and outside-root tests for permission requests.

Automated hermes-sweeper review; a human maintainer will make the final call.

malformed config. It lets Hermes safely answer Codex app-server permission
requests for roots the operator has already marked writable.
"""
config_path = Path(os.environ.get("CODEX_HOME", Path.home() / ".codex")) / "config.toml"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This bypasses the session's explicit codex_home override. CodexAppServerSession already stores that override and passes it to the app-server client, so parse writable roots from self._codex_home when present and only fall back to CODEX_HOME / ~/.codex otherwise.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/dashboard Web dashboard / control panel UI (dashboard/, landing) P2 Medium — degraded but workaround exists provider/openai OpenAI / Codex Responses API sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants