Skip to content

feat(proxy): add authenticated OpenAI Codex upstream - #62510

Open
dtownsel wants to merge 2 commits into
NousResearch:mainfrom
dtownsel:feat/openai-codex-proxy-luna
Open

feat(proxy): add authenticated OpenAI Codex upstream#62510
dtownsel wants to merge 2 commits into
NousResearch:mainfrom
dtownsel:feat/openai-codex-proxy-luna

Conversation

@dtownsel

Copy link
Copy Markdown
Contributor

Summary

  • add an openai-codex upstream adapter to the local OpenAI-compatible proxy
  • resolve and refresh existing Hermes Codex OAuth credentials without exposing them downstream
  • forward provider-required Codex headers with adapter-owned precedence
  • support configured downstream bearer authentication for Docker-accessible binds
  • refuse non-loopback startup unless --auth-token-file is configured

Security boundary

The downstream bearer is read from a file and compared in constant time. The downstream Authorization header is never forwarded to Codex; it is replaced with the refreshed upstream OAuth credential. Required provider headers override downstream-supplied values.

Validation

  • scripts/run_tests.sh tests/hermes_cli/test_proxy.py — 48 passed
  • ruff check hermes_cli/proxy hermes_cli/subcommands/gateway.py tests/hermes_cli/test_proxy.py — passed
  • live launchd-managed proxy smoke:
    • unauthenticated /v1/responses returned 401 proxy_auth_failed
    • authenticated Luna Responses stream returned exactly LAUNCHD_OK
    • health reported authenticated OpenAI Codex OAuth upstream

Deployment note

This PR does not add or store any OAuth credential or downstream token in the repository. Runtime secrets remain outside Git in a mode-0600 token file and Hermes's existing credential store.

@dtownsel

Copy link
Copy Markdown
Contributor Author

Independent review follow-up

A separate read-only reviewer audited the staged changes after implementation.

Verdict: no blocking security, correctness, or compatibility issues.

Verified by the reviewer:

  • downstream bearer enforcement is fail-closed for non-loopback binds;
  • downstream authorization is stripped and replaced with refreshed upstream OAuth;
  • adapter-owned headers take precedence;
  • no credential logging was found in the reviewed path;
  • retry behavior and the default Chat Completions path remain covered;
  • focused proxy suite: 48 passed.

Non-blocking note: bracketed or DNS loopback aliases are conservatively treated as non-loopback and therefore require a token file. This is intentionally safer than broad alias recognition, so no change was made.

@alt-glitch alt-glitch added type/feature New feature or request P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard area/auth Authentication, OAuth, credential pools provider/openai OpenAI / Codex Responses API sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Jul 11, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: competing open implementation alongside #54877 and #62297 — all three add an openai-codex upstream to the local hermes proxy but via different mechanisms (this PR: file-based downstream bearer + constant-time compare + loopback-refuse; #54877: reuses credential_pool/_codex_cloudflare_headers → /responses translation; #62297: dedicated Codex OAuth refresh + credential-pool selection + per-client key). Not a duplicate — flagged for a human to pick the canonical one.

@dtownsel

Copy link
Copy Markdown
Contributor Author

All local tests and live authenticated Luna canaries pass. The author has requested immediate merge; this PR is mergeable but branch protection requires a maintainer with merge permission.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for extending the existing proxy-adapter surface; current main still has no Codex proxy adapter (hermes_cli/proxy/adapters/__init__.py:16-19).

Problems

  • hermes_cli/proxy/adapters/openai_codex.py:59 cannot refresh a pool-only Codex credential after a 401. The resolver's pool fallback returns directly at hermes_cli/auth.py:3652-3666 without using force_refresh; the next line suppresses retry if that unchanged bearer is returned. The existing pool API provides try_refresh_current() (agent/credential_pool.py:1610-1621), and the xAI adapter uses that recovery pattern (hermes_cli/proxy/adapters/xai.py:76-109).
  • hermes_cli/proxy/cli.py:25 reads any path without checking file type or POSIX permissions, although the PR's security boundary relies on a mode-0600 downstream token file before allowing non-loopback binds.
  • The public proxy docs remain inconsistent with the new CLI behavior: website/docs/user-guide/features/subscription-proxy.md:167-174 documents unauthenticated LAN startup, and website/docs/reference/cli-commands.md:463 omits both Codex and --auth-token-file.

Suggested changes

  • Route pool-backed Codex recovery through selected-entry refresh/rotation and add a pool-only 401 regression test.
  • Reject insecure token files on POSIX and cover that failure mode.
  • Update both proxy documentation surfaces.

Automated hermes-sweeper review.

return None
with self._lock:
refreshed = self._resolve_credential(force_refresh=True)
if refreshed.bearer == failed_credential.bearer:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This cannot force-refresh a pool-only Codex credential: resolve_codex_runtime_credentials() returns directly from its pool fallback at hermes_cli/auth.py:3652-3666 without using force_refresh, so the unchanged bearer is rejected on the next line and no retry occurs. Select and refresh the pool entry here (as the xAI adapter does), and add a pool-only 401 regression test.

Comment thread hermes_cli/proxy/cli.py

def _read_downstream_token_file(path: str) -> str:
token_path = Path(path).expanduser()
token = token_path.read_text(encoding="utf-8").strip()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For a non-loopback bind, this accepts a token from any readable path, including a world-readable file or symlink. That does not enforce the stated mode-0600 security boundary. Validate a regular owner-only file on POSIX before allowing the bind, with an insecure-mode rejection test.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 11, 2026
@dtownsel

Copy link
Copy Markdown
Contributor Author

Follow-up containment fix added in 4830198 after live monitoring found gateway-internal subagent/compaction traffic reaching Honcho. The fix enforces the existing saveMessages=false hard gate in sync_turn, drops anchored internal gateway notifications, and refuses empty sanitized turns. Validation: 422 targeted Honcho/proxy tests passed; Ruff and git diff checks passed.

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

Labels

area/auth Authentication, OAuth, credential pools comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have 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/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants