Skip to content

fix(daemon): avoid bearer auth for ziti llm#125

Merged
rowan-stein merged 2 commits into
mainfrom
noa/issue-96
May 20, 2026
Merged

fix(daemon): avoid bearer auth for ziti llm#125
rowan-stein merged 2 commits into
mainfrom
noa/issue-96

Conversation

@casey-brooks
Copy link
Copy Markdown
Contributor

Summary

  • Fixes Instrument llm-proxy authorization mismatch causing chat e2e 403 chat-app#96.
  • Stops Codex agent turns from attaching a human OPENAI_API_KEY bearer token when the configured LLM endpoint is an OpenZiti host (*.ziti).
  • Omits Codex provider env_key for Ziti LLM endpoints so llm-proxy authenticates via workload/Ziti identity instead of resolving the human user token first.
  • Keeps the existing API-key behavior for non-Ziti/public LLM endpoints.
  • Updates github.com/agynio/codex-sdk-go from @main so Codex can launch without OPENAI_API_KEY in the Ziti path.

Evidence

Run 26183071285 logged principal_identity_type=user and tuple_user=identity:<human-user-id> for llm-proxy.ziti requests. Tracing found agynd was exporting the e2e human API token as OPENAI_API_KEY, and Codex provider config used env_key = "OPENAI_API_KEY", causing the bearer token to override Ziti identity resolution.

Tests

  • go test ./...
  • go vet ./...
  • go build ./...
  • git diff --check

@casey-brooks
Copy link
Copy Markdown
Contributor Author

Local validation

  • go test ./... — 190 passed / 0 failed / 0 skipped
  • go vet ./... — passed with no errors
  • go build ./... — passed
  • git diff --check — passed

Linting/static validation passed with no errors.

@casey-brooks
Copy link
Copy Markdown
Contributor Author

CI note

GitHub CI build passed. The PR e2e job did not reach tests; it failed during Deploy agynd-cli from source because the provisioned cluster did not contain the expected deployment:

WARNING: ArgoCD Application 'agynd' not found in argocd namespace.
Patching agynd deployment for DevSpace...
Error from server (NotFound): deployments.apps "agynd-agynd" not found

This is outside the local code path validated above; no agynd tests ran in that failed e2e job.

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Starting review.

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Requesting changes for one auth-path issue: the Ziti path still needs to guarantee Codex cannot inherit OPENAI_API_KEY from the agynd process environment.

Comment thread internal/daemon/codexconfig.go
@casey-brooks
Copy link
Copy Markdown
Contributor Author

@Noa-Licent addressed the requested auth-path change in commit 46b1ffb.

What changed:

  • For *.ziti LLM endpoints, agynd now scrubs parent-process Codex auth env vars before calling codex.NewClient, so codex-sdk-go cannot copy inherited auth into the Codex process env:
    • OPENAI_API_KEY
    • CODEX_API_KEY
    • CODEX_ACCESS_TOKEN
  • The scrub is scoped to Codex client startup and restores agynd's parent env afterward.
  • Non-Ziti/public LLM endpoints still keep the existing OPENAI_API_KEY behavior.
  • Added regression coverage that sets parent auth env vars and asserts they are absent during the Ziti Codex startup path and absent from the Ziti Codex env/config.

Local validation:

  • go test ./... — 192 passed / 0 failed / 0 skipped
  • go vet ./... — passed with no errors
  • go build ./... — passed
  • git diff --check — passed

Ready for re-review.

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Re-reviewing commit 46b1ffb.

Copy link
Copy Markdown

@noa-lucent noa-lucent left a comment

Choose a reason for hiding this comment

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

Re-review complete. Commit 46b1ffb addresses the prior auth-path concern by scrubbing inherited Codex auth environment variables during Ziti Codex startup, restoring them afterward, and adding regression coverage for parent env inheritance plus Ziti config/env omission. I resolved my prior thread. CI build is passing; e2e is still in progress at review time.

@rowan-stein rowan-stein merged commit e98e384 into main May 20, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Instrument llm-proxy authorization mismatch causing chat e2e 403

3 participants