Skip to content

feat(accounts): add per-account OAuth proxy support#804

Open
rupebac wants to merge 9 commits into
Soju06:mainfrom
rupebac:feature/account-egress-isolation
Open

feat(accounts): add per-account OAuth proxy support#804
rupebac wants to merge 9 commits into
Soju06:mainfrom
rupebac:feature/account-egress-isolation

Conversation

@rupebac
Copy link
Copy Markdown

@rupebac rupebac commented May 26, 2026

Summary

Adds per-account SOCKS5 proxy support for OAuth account import and account-bound upstream egress, so proxied accounts are validated before persistence and continue using their configured proxy for
refreshes, usage, model fetches, files, HTTP responses, and WebSocket traffic.

Type of change

  • feat: — new user-facing feature or capability
  • fix: — bug fix (no behavior change beyond the bug)
  • refactor: — internal refactor (no behavior change, no API change)
  • docs: — documentation only
  • chore: / ci: / build: — tooling, CI, packaging
  • test: — test-only change
  • Breaking change (also append ! after the type, e.g. feat!: or include BREAKING CHANGE: footer)

Linked issue:

OpenSpec

  • This PR includes / updates an OpenSpec change
  • Not applicable — bug fix that matches the existing spec
  • Not applicable — docs / CI / chore only
  • This PR touches a codex-faithful path (image pipeline, request/response
    shape, SSE framing, OAuth flow) and preserves upstream-equivalent behavior

Change directory: openspec/changes/add-oauth-account-proxy/

Changes

  • Adds nullable per-account proxy columns to accounts, plus repository/service/API contracts for storing, validating, updating, and clearing account proxy config.
  • Adds account-bound HTTP/WebSocket client handling so proxied accounts use SOCKS5 egress, while direct accounts use isolated per-account direct sessions instead of the shared global client.
  • Updates OAuth add-account flow to defer persistence when proxy config is supplied, validate the proxy with upstream token refresh/account calls, and avoid creating an unproxied active account on
    failure.
  • Adds dashboard UI for proxy config during OAuth/import and account detail editing, including proxy error handling and password-preservation behavior.
  • Keeps browser OAuth redirect_uri tied to the registered configured callback and reuses that same URI for token exchange.
  • Adds OpenSpec requirements/context for account egress proxy behavior and outbound client isolation.

Test plan

uv run ruff check
# All checks passed!

uv run pyright app/
# 0 errors, 1 warning, 0 informations

uv run pytest tests/integration/test_migrations.py -q
# 12 passed, 3 skipped

uv run pytest tests/integration/test_oauth_flow.py::test_browser_oauth_redirect_uses_registered_uri_and_matches_token_exchange -q
# 1 passed

openspec validate add-oauth-account-proxy --strict
# Change 'add-oauth-account-proxy' is valid

openspec validate --specs
# Totals: 22 passed, 0 failed (22 items)

Earlier broader verification also passed:

uv run pytest tests/integration/test_oauth_flow.py tests/integration/test_accounts_proxy_api.py tests/integration/test_account_proxy_repository.py tests/integration/test_migrations.py tests/unit/
test_account_http_client.py tests/unit/test_account_proxy_failures.py tests/unit/test_account_proxy_probe.py tests/unit/test_account_proxy_schemas.py tests/unit/test_account_tls.py -q
# 117 passed, 3 skipped

cd frontend && npm run typecheck
# passed

cd frontend && npm test -- --run src/features/accounts/oauth-dialog.test.tsx src/features/accounts/hooks/use-oauth.test.ts src/features/accounts/proxy.test.tsx src/features/accounts/proxy-
errors.test.ts src/features/accounts/components/import-dialog.test.tsx
# 40 tests passed

Live verification:

- Deployed branch to the test host.
- Added a proxied OAuth account successfully.
- Confirmed the account persisted as active with proxy metadata and no tls_profile field.
- Confirmed invalid proxy credentials fail before account persistence with Proxy validation failed: proxy_auth.

## Screenshots / output (optional)

Live OAuth/proxy validation was exercised manually on the deployed test instance. No screenshots attached.

## Checklist

- [x] Title is in Conventional Commits format (<type>(<scope>)?: <subject>).
- [ ] Linked the related issue / discussion above.
- [x] Added or updated tests covering the change.
- [x] Ran uv run pre-commit run local-ci --hook-stage manual --all-files or the relevant make <target> subset locally.
- [x] If touching specs: openspec validate --specs passes and /opsx:verify is clean.
- [x] CHANGELOG is not edited by hand (release-please handles it).

@rupebac rupebac requested a review from Soju06 as a code owner May 26, 2026 00:49
@github-actions github-actions Bot added the db migration PR changes Alembic database migrations; maintainer must coordinate merge order label May 26, 2026
@gitguardian
Copy link
Copy Markdown

gitguardian Bot commented May 26, 2026

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them.
While these secrets were previously flagged, we no longer have a reference to the
specific commits where they were detected. Once a secret has been leaked into a git
repository, you should consider it compromised, even if it was deleted immediately.
Find here more information about risks.


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

@rupebac rupebac force-pushed the feature/account-egress-isolation branch from e8a3041 to 2f42a42 Compare May 26, 2026 00:56
@rupebac
Copy link
Copy Markdown
Author

rupebac commented May 26, 2026

this has been purely vibed coded, but it has been working great for me

@Komzpa Komzpa force-pushed the feature/account-egress-isolation branch 5 times, most recently from 38eaa03 to c8be21b Compare May 26, 2026 17:11
@Komzpa
Copy link
Copy Markdown
Collaborator

Komzpa commented May 26, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8be21b907

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/modules/oauth/service.py Outdated
Comment thread app/modules/accounts/repository.py Outdated
@Soju06 Soju06 added the 🤖 codex: needs work [@codex review] raised an issue label May 26, 2026
@Komzpa Komzpa force-pushed the feature/account-egress-isolation branch from c8be21b to e58d3e6 Compare May 26, 2026 17:46
@Soju06 Soju06 removed the 🤖 codex: needs work [@codex review] raised an issue label May 26, 2026
@Komzpa Komzpa force-pushed the feature/account-egress-isolation branch from e58d3e6 to c8e031b Compare May 26, 2026 17:48
@Komzpa
Copy link
Copy Markdown
Collaborator

Komzpa commented May 26, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@Komzpa
Copy link
Copy Markdown
Collaborator

Komzpa commented May 26, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@Soju06
Copy link
Copy Markdown
Owner

Soju06 commented May 27, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@rupebac
Copy link
Copy Markdown
Author

rupebac commented May 28, 2026

This started as a local spike, but I have now rebased it onto the latest PR head and prepared a focused follow-up commit (947924a, pending push to this PR branch).

Current follow-up scope:

  • Token refresh jitter

    • Changed per-account refresh jitter from a signed offset (interval ± jitter) to an early-only offset (interval - jitter).
    • This preserves spreading across accounts while keeping token_refresh_interval_days as the hard maximum token age.
    • Added coverage that jitter stays within [0, jitter] and never delays accounts beyond the configured interval.
  • Targeted re-authentication

    • The Accounts UI now passes the selected account id when clicking re-auth.
    • OAuth state carries that target account id through browser, manual callback, and device-code flows.
    • Successful OAuth writes the new tokens back into the selected existing account row instead of going through generic add-account/upsert behavior.
    • Existing proxy configuration is preserved.
    • If the selected account has a stored proxy, server-side OAuth bootstrap/token-exchange calls use that stored proxy during re-auth.
    • Re-auth rejects returned credentials for a different ChatGPT account id/email instead of overwriting the selected account.
    • If the stored proxy password cannot be decrypted, re-auth fails explicitly rather than silently falling back to no proxy.
  • Browser OAuth redirect regression

    • Live re-auth testing exposed that browser OAuth was rewriting the configured callback from http://localhost:1455/auth/callback to the remote dashboard host, e.g. http://192.168.x.x:1455/auth/callback.
    • The previous working image did not rewrite this URI, and OpenSpec already says browser OAuth must use the configured oauth_redirect_uri unchanged.
    • Restored the registered callback behavior and updated the integration test that had drifted to expect the rewrite.
  • Diagnostics

    • Added refresh-failure logging with account id, request id, HTTP status, upstream error code, and message.
    • Added logging before deactivating an account due to permanent token refresh failure.
  • OpenSpec

    • Added fix-token-refresh-jitter-max-age.
    • Added fix-oauth-reauth-preserve-proxy.
    • Synced resulting requirements into the relevant main specs.

Validation run locally:

  • uv run ruff check ...
  • uv run pytest tests/integration/test_oauth_flow.py -q -> 40 passed
  • broader targeted backend pytest run -> 144 passed
  • npm run test -- src/features/accounts/hooks/use-oauth.test.ts
  • npm run typecheck
  • npm run lint -- ...
  • npm run build
  • openspec validate --specs
  • openspec validate fix-token-refresh-jitter-max-age --strict
  • openspec validate fix-oauth-reauth-preserve-proxy --strict

@SHAREN
Copy link
Copy Markdown
Contributor

SHAREN commented May 31, 2026

Hi. Do you have any rough estimate of how much proxy bandwidth is used when per-account proxy is enabled?

For example, if an account consumes around 100 million tokens, how much traffic would go through the proxy approximately? And if usage is around 500 million tokens per week, what proxy bandwidth should I plan for?

I understand it depends on request/response size, streaming overhead, retries, files, etc. I only need an approximate order of magnitude.

@rupebac
Copy link
Copy Markdown
Author

rupebac commented May 31, 2026

for me working a lot, I use less than 1gb every week (with 2 accounts per proxy)... but I am having an issue with this right now... accounts get deactivated every 2 days or so and need to re-auth... not sure what yet, keep doing tests... I have deactivated the codex tls profile, still the same.. rotate usage fetch's so they do not match at the same time, and the same... keep doing more tests, will inform.

@Soju06
Copy link
Copy Markdown
Owner

Soju06 commented May 31, 2026

@codex review

1 similar comment
@Soju06
Copy link
Copy Markdown
Owner

Soju06 commented May 31, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a7f46409b1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/core/clients/proxy_websocket.py Outdated
@Komzpa Komzpa added the 🤖 codex: needs work [@codex review] raised an issue label May 31, 2026
@Komzpa Komzpa force-pushed the feature/account-egress-isolation branch from a7f4640 to dfd585f Compare May 31, 2026 21:47
@Komzpa Komzpa force-pushed the feature/account-egress-isolation branch from dfd585f to b3afa15 Compare June 1, 2026 04:10
@Soju06
Copy link
Copy Markdown
Owner

Soju06 commented Jun 1, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59d73dee2c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/modules/oauth/service.py Outdated
@Komzpa Komzpa added the 🤖 codex: needs work [@codex review] raised an issue label Jun 1, 2026
@Soju06
Copy link
Copy Markdown
Owner

Soju06 commented Jun 1, 2026

Hermes owner-review found current-head blockers that should be addressed before merge:

  1. Credential leak on websocket proxy errorsapp/core/clients/account_http.py builds account websocket proxy URIs with username:password@host, and app/core/clients/proxy_websocket.py returns str(InvalidProxy) directly in the OpenAI error envelope. websockets.exceptions.InvalidProxy("socks5h://user:secret@proxy.example.com:1080", "bad") renders the full URI including credentials, so malformed/unsupported websocket proxy errors can leak the stored proxy password to the downstream caller. Please redact/sanitize proxy URIs before client-visible errors/logs and add a regression.

  2. Proxy edit UI/API password semantics are misleading — the frontend/API schemas say omitted password keeps the existing password, and the edit dialog defaults to Keep, but AccountsService.set_account_proxy() only reuses the old password when host/port/username are unchanged. If an operator leaves Keep selected while changing the proxy host/port/user, the backend can persist the new proxy with hasPassword=false after a no-auth probe. Please either require Replace when the proxy identity changes or make the UI/API wording explicit and prevent silent password clearing.

  3. Stable OpenSpec contradictionopenspec/specs/account-egress-proxy/spec.md requires import-with-proxy to validate and atomically persist proxy fields, while the later “Auth.json import/export does not carry proxy configuration” requirement says import/export endpoints MUST NOT read or write proxy configuration and proxy is managed only through dedicated proxy endpoints. Please narrow the latter to “proxy fields inside the auth.json file are ignored/export does not include proxy state,” or otherwise reconcile the normative requirements.

Additional note: the current non-outdated Codex thread about reauthAccountId + expectProxy looks addressed/unreachable because start_oauth() now rejects that combination before the tokens_ready finalization path.

@Soju06 Soju06 added hermes: needs-followup Hermes left a blocker/comment that needs follow-up observation and removed 🤖 codex: needs work [@codex review] raised an issue labels Jun 1, 2026
@Soju06
Copy link
Copy Markdown
Owner

Soju06 commented Jun 1, 2026

@codex review

@Komzpa Komzpa force-pushed the feature/account-egress-isolation branch 2 times, most recently from c0bc941 to 2bcdb27 Compare June 1, 2026 06:55
@Soju06
Copy link
Copy Markdown
Owner

Soju06 commented Jun 1, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Soju06 Soju06 added the 🤖 codex: ok [@codex review] says no issues found. label Jun 1, 2026
@Komzpa Komzpa removed the hermes: needs-followup Hermes left a blocker/comment that needs follow-up observation label Jun 1, 2026
@Soju06 Soju06 removed the 🤖 codex: ok [@codex review] says no issues found. label Jun 1, 2026
@Komzpa Komzpa force-pushed the feature/account-egress-isolation branch from 2bcdb27 to 0770ed5 Compare June 1, 2026 14:03
@rupebac
Copy link
Copy Markdown
Author

rupebac commented Jun 1, 2026

for me working a lot, I use less than 1gb every week (with 2 accounts per proxy)... but I am having an issue with this right now... accounts get deactivated every 2 days or so and need to re-auth... not sure what yet, keep doing tests... I have deactivated the codex tls profile, still the same.. rotate usage fetch's so they do not match at the same time, and the same... keep doing more tests, will inform.

Did you find more something about this? I'm having the same here but without proxy. I started think if isolated every account in one proxy would avoid this to happen...

uhmm not yet... the thing about proxy per account is that you can´t reuse the connection for usage fetch across all accounts, and stuff like that, as that defeats the purpose of the proxy itself and would connect accounts... also, this was stripping out the installation id header from codex, as that would also connect accounts (even through proxy)... the test I am reproducing now is generating a random installation id per account, as I thought that sending requests without one may be the reason as well. Also, fetching usage of all accounts at the same time is sort of connecting accounts (also addressed locally in my latest changes). But only 12h since deployed, usually for me they were lasting 2-3 days... so I need to wait a bit more... I do have 1 personal account (without proxy) that is never expiring, so there is some sort of reputation taking place I think... I am testing with ISP Dedicated proxies from iproyal.... It could be that the reputation of these IPs is damaged already not sure... If there are more ideas, please let me know :)

@Komzpa
Copy link
Copy Markdown
Collaborator

Komzpa commented Jun 1, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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

Labels

db migration PR changes Alembic database migrations; maintainer must coordinate merge order

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants