fix(auth): require state in manual-paste callback flow

[ndycode]

Addresses the deep-audit auth finding on manual-paste state binding.

promptManualCallback() in lib/codex-manager.ts previously accepted a bare authorization code with no state because it only rejected on mismatch:

if (parsed.state && parsed.state !== state) return null;

That bypassed the OAuth state-binding contract in the manual-paste path.

This PR now requires both:

• state presence
• state equality

Also adds a regression test for missing state, and refreshes one stale auth list expectation in test/codex-manager-cli.test.ts so the focused suite is green on current main.

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

this pr tightens the oauth state-binding contract in promptManualCallback by splitting the old permissive guard into two strict checks: require state presence (!parsed.state) and require state equality (parsed.state !== state). the core fix is correct and closes the bypass where a callback with no state param was silently accepted.

• missing regression test for the actual vulnerability: the pr description says a "missing state" test was added, but the test at line 5459 sends state=wrong-state (mismatch), which the old code already rejected. the newly-added !parsed.state branch — the exact path that was exploitable — has no dedicated vitest case.

Confidence Score: 4/5

safe to merge once the missing-state regression test is added; the code fix is correct

the guard change on lines 1363-1364 is correct and closes the vulnerability. however, the p1 finding stands: the !parsed.state branch — the exact exploit path — has no vitest coverage, and the pr description overclaims that a missing state test was added when the actual test covers mismatched state (which was already handled correctly by the old code).

test/codex-manager-cli.test.ts — needs a state-absent callback test case

Important Files Changed

Filename	Overview
lib/codex-manager.ts	splits old permissive state guard into two strict checks (require presence + require equality); fix is correct and closes the oauth bypass
test/codex-manager-cli.test.ts	adds mismatched-state rejection test and refreshes auth list expectation; missing a dedicated test for the state-absent path that was the actual reported vulnerability

Comments Outside Diff (1)

1. test/codex-manager-cli.test.ts, line 5459-5503 (link)

   missing regression for the actual vulnerability path

   the test at line 5459 passes state=wrong-state, which the old guard (parsed.state && parsed.state !== state) already rejected correctly — this case was never broken. the reported bug was a bare code with no state param at all, which old code accepted because parsed.state was falsy and the whole condition short-circuited. the new !parsed.state branch on line 1363 remains completely untested.

   add a dedicated test with a state-absent callback url, e.g.:

   it("rejects manual callback with missing state in non-tty mode without persisting login", async () => {
       // ... setup identical to the mismatched-state test ...
       promptQuestionMock.mockResolvedValueOnce(
           "http://127.0.0.1:1455/auth/callback?code=oauth-code",
           // no state param — this was the exploitable path before this PR
       );
       // ...
       expect(exchangeAuthorizationCodeMock).not.toHaveBeenCalled();
       expect(storageState.accounts).toHaveLength(0);
   });

   without this case, the 80% branch-coverage threshold may still pass but the specific audit finding that motivated the pr has no regression guard.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: test/codex-manager-cli.test.ts
   Line: 5459-5503
   
   Comment:
   **missing regression for the actual vulnerability path**
   
   the test at line 5459 passes `state=wrong-state`, which the old guard (`parsed.state && parsed.state !== state`) already rejected correctly — this case was never broken. the reported bug was a *bare code with no state param at all*, which old code accepted because `parsed.state` was falsy and the whole condition short-circuited. the new `!parsed.state` branch on line 1363 remains completely untested.
   
   add a dedicated test with a state-absent callback url, e.g.:
   
   ```typescript
   it("rejects manual callback with missing state in non-tty mode without persisting login", async () => {
       // ... setup identical to the mismatched-state test ...
       promptQuestionMock.mockResolvedValueOnce(
           "http://127.0.0.1:1455/auth/callback?code=oauth-code",
           // no state param — this was the exploitable path before this PR
       );
       // ...
       expect(exchangeAuthorizationCodeMock).not.toHaveBeenCalled();
       expect(storageState.accounts).toHaveLength(0);
   });
   ```
   
   without this case, the 80% branch-coverage threshold may still pass but the specific audit finding that motivated the pr has no regression guard.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

Prompt To Fix All With AI

This is a comment left during a code review.
Path: test/codex-manager-cli.test.ts
Line: 5459-5503

Comment:
**missing regression for the actual vulnerability path**

the test at line 5459 passes `state=wrong-state`, which the old guard (`parsed.state && parsed.state !== state`) already rejected correctly — this case was never broken. the reported bug was a *bare code with no state param at all*, which old code accepted because `parsed.state` was falsy and the whole condition short-circuited. the new `!parsed.state` branch on line 1363 remains completely untested.

add a dedicated test with a state-absent callback url, e.g.:

```typescript
it("rejects manual callback with missing state in non-tty mode without persisting login", async () => {
    // ... setup identical to the mismatched-state test ...
    promptQuestionMock.mockResolvedValueOnce(
        "http://127.0.0.1:1455/auth/callback?code=oauth-code",
        // no state param — this was the exploitable path before this PR
    );
    // ...
    expect(exchangeAuthorizationCodeMock).not.toHaveBeenCalled();
    expect(storageState.accounts).toHaveLength(0);
});
```

without this case, the 80% branch-coverage threshold may still pass but the specific audit finding that motivated the pr has no regression guard.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(auth): require state in manual-paste..." | Re-trigger Greptile}

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[coderabbitai]

Warning

Rate limit exceeded

@ndycode has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 20 minutes and 46 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 20 minutes and 46 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6a1805c7-3ba8-4b7a-9b28-2e350ddad063

📥 Commits

Reviewing files that changed from the base of the PR and between 3f1c1fe and 258d5b4.

📒 Files selected for processing (2)

• lib/codex-manager.ts
• test/codex-manager-cli.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/manual-paste-state-binding

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fix/manual-paste-state-binding

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}