Skip to content

fix(codex): reject stale low-quota policy evidence - #677

Closed
luvs01 wants to merge 2 commits into
devfrom
codex/propose-fix-for-stale-quota-vulnerability
Closed

luvs01 wants to merge 2 commits into
devfrom
codex/propose-fix-for-stale-quota-vulnerability

Conversation

@luvs01

@luvs01 luvs01 commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Motivation

  • Prevent a delayed HTTP quota response captured with a retired pool credential from durably pausing a replacement account that reuses the same pool ID.

Description

  • Require that any captured historyEvidence writer remains live before appending history and before invoking observeCodexLowQuota by introducing a livePoolEvidence verdict in setAccountQuotaFromParsed (src/codex/quota.ts).
  • Preserve existing display quota updates and snapshot publication while gating only the policy/observer path on writer liveness so visual telemetry remains unaffected.
  • Add a regression test that captures a pool writer, removes and recreates the account under the same ID, applies a delayed retired-writer quota response, and verifies the replacement is not paused (tests/codex-integration/low-quota-protection.test.ts).
  • Document the invariant that pool quota policy actions require a currently live credential-bound writer and that retired-credential responses cannot authorize pauses (structure/providers/openai-accounts.md).

Testing

  • Ran the focused integration tests with the pinned Bun runtime using ./node_modules/.bin/bun test tests/codex-integration/low-quota-protection.test.ts and all tests in that file passed (24 pass, 0 fail).
  • Ran ./node_modules/.bin/bun run typecheck, ./node_modules/.bin/bun run structure:check, and ./node_modules/.bin/bun run privacy:scan, and each completed successfully.
  • bun test using the system Bun (1.2.14) failed due to a missing node:zlib.zstdDecompressSync export, but the repository-pinned Bun 1.4.0 used for the focused run succeeded.
  • ./node_modules/.bin/bun run test:changed could not run in this checkout because there is no dev/remote dev comparison ref; focused coverage and static checks above were executed instead.

Codex Task


Devin Review

Summary by CodeRabbit

  • Bug Fixes
    • Quota responses from pooled accounts now trigger low-quota pauses only when backed by a valid, live credential. Responses from replaced credentials or without captured credential evidence can still refresh the displayed quota but won’t pause the account.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: luvs01/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 2b7a8730-913f-4015-a6c7-827b2c7a20e3

📥 Commits

Reviewing files that changed from the base of the PR and between 37ad7e7 and 378348f.

📒 Files selected for processing (8)
  • src/codex/auth-api/pool-quota-probe.ts
  • src/codex/quota-auto-refresh.ts
  • src/codex/quota.ts
  • src/server/responses/compact.ts
  • src/server/responses/core-codex-account.ts
  • src/server/responses/passthrough-delivery.ts
  • structure/providers/openai-accounts.md
  • tests/codex-integration/low-quota-protection.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Quota updates now identify pool responses separately from writer-free observations. Pool history and low-quota observations require matching, live writer evidence. Call sites pass pool-response context, and tests cover retired and uncaptured writers.

Changes

Pool quota observations

Layer / File(s) Summary
Quota evidence and observation rules
src/codex/quota.ts
setAccountQuotaFromParsed and applyAccountQuotaFromUpstreamHeaders now carry pool-request context. Pool history and low-quota observation require matching, live writer evidence.
Pool response context at callers
src/codex/auth-api/pool-quota-probe.ts, src/codex/quota-auto-refresh.ts, src/server/responses/compact.ts, src/server/responses/core-codex-account.ts, src/server/responses/passthrough-delivery.ts, tests/codex-integration/low-quota-protection.test.ts, structure/providers/openai-accounts.md
Pool-response call sites pass the new context. Tests verify that responses from retired credentials or without a captured writer do not pause the account. Documentation describes the live-writer condition.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: lidge-jun

Merge Risk: ⚪ Minimal · up to 37834

No actionable merge-blocking risk was established. The pool quota paths preserve the live-writer condition while retaining the documented login behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preventing stale low-quota policy evidence from affecting replacement pool accounts. It matches the pull request objectives and changed files…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

devin-ai-integration[bot]

This comment was marked as resolved.

@github-actions github-actions Bot added the bug Something isn't working label Sep 29, 2026
@github-actions

Copy link
Copy Markdown

✅ Deterministic PR hygiene checks passed.

…iter

A pool response whose credential-bound writer capture failed before dispatch used to reach policy observation as if it were writer-free legacy/login evidence. The request now carries its pool provenance separately, so an absent writer after a credential replacement can no longer pause the replacement account.
@luvs01 luvs01 closed this Sep 30, 2026
@luvs01

luvs01 commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

Retrospective closure record (2026-10-05): this explanation is being added after the original close.

The implementation at 378348f446c5dd8d3ad29f4e0749668341c0b987 was carried into landed commit b0d275dc79e8ce89de3495a60cbad6bb8b7ee5e2, an ancestor of checked dev 829a18ba94941aa7997d34cbd88f97390844d79b. This conclusion is based on the actual runtime changes, not a claim that the original commit is itself an ancestor of the carried head.

The carried code preserves live pool-writer validation and propagates poolRequest/poolResponse provenance through WHAM, warmup/header handling, compact, WebSocket and normal Responses delivery. src/codex/quota-auto-refresh.ts matches blob 42fa040ee53d6af07bf8bce5d4a4267214dfbe89 in the original and carried changes. The quota boundary uses live evidence to gate history and low-quota policy observations; additional tests were carried as well.

This duplicate proposal remains closed because its implementation has landed. No independent missing change was found that would justify reopening or repushing this historical head.

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

Labels

aardvark bug Something isn't working codex

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant