Skip to content

feat(proxy): route security work to authorized accounts#762

Closed
Komzpa wants to merge 14 commits into
Soju06:mainfrom
Komzpa:revive/pr-572
Closed

feat(proxy): route security work to authorized accounts#762
Komzpa wants to merge 14 commits into
Soju06:mainfrom
Komzpa:revive/pr-572

Conversation

@Komzpa
Copy link
Copy Markdown
Collaborator

@Komzpa Komzpa commented May 21, 2026

Summary

Revives #572 on a maintainer-owned branch because the original PR head has maintainerCanModify=false.

Credit: original implementation by @Komzpa in #572; this branch rebases and updates that work on current main.

  • Adds security/cyber-flagged account routing with an account-level authorization toggle and proxy fallback advisory behavior.
  • Removes old stack bleed, preserves additional quota flags for security routing, and fixes websocket gate/error handling around security replays.

Validation

  • openspec validate add-security-work-account-routing --strict passed
  • uv run ruff check ... passed
  • uv run ty check app/modules/accounts app/modules/proxy passed
  • uv run pytest ... -k 'security_work or security_retry' passed
  • bun run test src/features/accounts/components/accounts-page.test.tsx passed
  • uv run python -m app.db.migrate ... upgrade && ... check passed: migration_policy=ok, schema_drift=none
  • git diff --check origin/main..HEAD passed

OpenSpec

  • add-security-work-account-routing remains included and valid.

Revives #572

@Soju06
Copy link
Copy Markdown
Owner

Soju06 commented May 21, 2026

@codex review

@Komzpa Komzpa added the db migration PR changes Alembic database migrations; maintainer must coordinate merge order label May 21, 2026
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

ℹ️ 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 May 21, 2026
@Soju06 Soju06 removed the 🤖 codex: ok [@codex review] says no issues found. label May 22, 2026
@KakatkarAkshay
Copy link
Copy Markdown

Maybe worth considering: for normal work, we could prefer regular accounts first and save the security-authorized accounts for requests that actually need them. That might help preserve usage on those accounts.

@Komzpa
Copy link
Copy Markdown
Collaborator Author

Komzpa commented May 22, 2026

@KakatkarAkshay I think it's best to manage it separately, see #761 - maybe it's authorized but you plan to get rid of it so burn first, or something like that.

@KakatkarAkshay
Copy link
Copy Markdown

@KakatkarAkshay I think it's best to manage it separately, see #761 - maybe it's authorized but you plan to get rid of it so burn first, or something like that.

Ah, I hadn’t seen #761 earlier. That makes sense, keeping this policy separate there sounds good.

@Soju06
Copy link
Copy Markdown
Owner

Soju06 commented May 22, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

ℹ️ 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 🤖 codex: ok [@codex review] says no issues found. and removed 🤖 codex: ok [@codex review] says no issues found. labels May 22, 2026
@Komzpa
Copy link
Copy Markdown
Collaborator Author

Komzpa commented May 24, 2026

@codex review

1 similar comment
@Soju06
Copy link
Copy Markdown
Owner

Soju06 commented May 24, 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: 153458f20a

ℹ️ 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/proxy/service.py
@Komzpa Komzpa added the 🤖 codex: needs work [@codex review] raised an issue label May 24, 2026
@Komzpa
Copy link
Copy Markdown
Collaborator Author

Komzpa commented May 24, 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".

@Komzpa Komzpa added 🤖 codex: ok [@codex review] says no issues found. and removed 🤖 codex: needs work [@codex review] raised an issue labels May 24, 2026
@Soju06 Soju06 removed the 🤖 codex: ok [@codex review] says no issues found. label May 26, 2026
@Komzpa Komzpa force-pushed the revive/pr-572 branch 2 times, most recently from c22cc9c to 00679ab Compare May 26, 2026 15:15
@Soju06
Copy link
Copy Markdown
Owner

Soju06 commented May 26, 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".

@Soju06 Soju06 added the 🤖 codex: needs work [@codex review] raised an issue label May 31, 2026
@Soju06 Soju06 removed the 🤖 codex: needs work [@codex review] raised an issue label May 31, 2026
@Komzpa
Copy link
Copy Markdown
Collaborator Author

Komzpa commented May 31, 2026

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

ℹ️ 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 May 31, 2026
@Soju06
Copy link
Copy Markdown
Owner

Soju06 commented Jun 1, 2026

Hermes blocker review for current head f944ecd57f5b710f9512e5e01c2300ecd66f301c:

app/modules/proxy/service.py still lets direct HTTP streamed Responses security-work retry move a previous_response_id request to another account when owner resolution misses.

Evidence:

  • service.py:9876-9883 only sets require_preferred_account=True when _resolve_websocket_previous_response_owner() returns an account. If a previous_response_id is present but the owner lookup misses, the request is no longer treated as pinned.
  • The security-work retry branches at service.py:10301-10325, service.py:10419-10449, and service.py:10681-10704 only guard require_preferred_account / file-pinning, not payload.previous_response_id is not None.
  • The retry then calls _stream_once(account, payload, ...) with the original payload at service.py:10217-10223, so the old previous_response_id is sent to the newly selected security-authorized account.
  • The OpenSpec delta says security-work retry is allowed only when the request can be safely replayed, and that previous-response-pinned requests must not move accounts (openspec/changes/add-security-work-account-routing/specs/responses-api-compat/spec.md:3-25). WebSocket has the needed fresh-payload guard (service.py:8758-8764), but the direct HTTP stream path does not.

Please fail closed for any direct HTTP stream request with payload.previous_response_id unless the retry path has an explicitly safe fresh/full-resend payload and removes the anchor before selecting another account. Add a regression test for this direct stream path; the existing previous-response security tests cover HTTP-bridge/WebSocket helpers, not this _stream_once(payload) path.

@Soju06 Soju06 added the hermes: needs-followup Hermes left a blocker/comment that needs follow-up observation 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
@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: 63e7036438

ℹ️ 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/proxy/load_balancer.py
@Komzpa Komzpa added the 🤖 codex: needs work [@codex review] raised an issue label Jun 1, 2026
@Komzpa Komzpa removed the 🤖 codex: needs work [@codex review] raised an issue label Jun 1, 2026
@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: 2cdb70457f

ℹ️ 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/proxy/service.py
@Komzpa Komzpa added the 🤖 codex: needs work [@codex review] raised an issue label Jun 1, 2026
@Soju06 Soju06 removed the 🤖 codex: needs work [@codex review] raised an issue label Jun 1, 2026
@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. What shall we delve into next?

ℹ️ 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 added a commit to Komzpa/codex-lb that referenced this pull request Jun 1, 2026
# Conflicts:
#	app/modules/accounts/mappers.py
#	app/modules/proxy/service.py
#	frontend/src/features/accounts/components/account-actions.tsx
#	frontend/src/features/accounts/components/account-detail.tsx
#	frontend/src/features/accounts/components/account-list-item.tsx
#	frontend/src/features/accounts/components/accounts-page.tsx
#	frontend/src/test/mocks/handlers.ts
@Komzpa
Copy link
Copy Markdown
Collaborator Author

Komzpa commented Jun 2, 2026

Folded into #716.

#716 is the surviving main-based routing/quota branch for this bucket. It includes the routing/quota changes from this PR, has current-head CI green, and has a clean current-head Codex review, so this separate PR can be closed to keep the queue reviewable.

@Komzpa Komzpa closed this Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖 codex: ok [@codex review] says no issues found. 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.

3 participants