🔒 fix: use json instead of ast.literal_eval for state data

[undivisible]

🎯 What: The vulnerability fixed was the use of ast.literal_eval to parse state_data_str when retrieving OAuth state data from Redis.
⚠️ Risk: While safer than eval(), ast.literal_eval can still cause a denial-of-service (crashing the process) if provided deeply nested structures.
🛡️ Solution: Changed the serialization of state_data to use json.dumps and deserialization to use json.loads. Also updated the mocked redis payloads in tests to match the new json.dumps format. In-flight pre-migration str(dict) payloads (single quotes, 10-minute TTL) are still accepted by mapping them to JSON.

Failure-Class: none

---

PR created automatically by Jules for task 5719183998940294315 started by @undivisible

---

Note

Low Risk
Narrow change to OAuth state serialization in task integrations; atomic consume behavior is unchanged, with a bounded legacy parse fallback during the 10-minute TTL window.

Overview
OAuth CSRF state in task_integrations is now written to Redis with json.dumps and read back with json.loads, replacing str(dict) + ast.literal_eval.

validate_and_consume_oauth_state still uses atomic Redis GETDEL for single-use consumption; parsing now rejects non-dict payloads and includes a short backward-compat path for in-flight keys stored as Python repr (single-quote str(dict) → quote swap + json.loads, no ast.literal_eval). Unit tests were updated so Redis fixtures use JSON bytes instead of repr(...).

^{Reviewed by Cursor Bugbot for commit f80686a. Configure here.}

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-31T16:51:44.427235Z	9406559	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9406559a16

ℹ️ 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".

[Git-on-my-level]

Thanks for this — the core change is right, and it brings task_integrations.py in line with the sibling OAuth state handling in routers/integrations.py, which already serializes with json.dumps/json.loads. Two things need fixing before merge:

Blocking

1. Stray pr_body.md committed at the repo root. The failure-class contract is evaluated against the PR description (the preflight pulls it from the GitHub API), never against a file in the diff — so this file doesn't satisfy the check and would land in the repo root on main. Please delete it from the branch and add Failure-Class: none on its own line in the PR description instead. That's exactly what the failing Hygiene job (failure-class-protocol → missing_declaration) is asking for: all three commits have fix: subjects and the current description has no declaration. The next push re-runs Hygiene with both fixes in place.
2. Stale docstring in backend/tests/unit/test_task_integrations_oauth_atomic.py — lines 6–7 still read "(This handler parses the stored value with ast.literal_eval, so the fixture stores a Python repr rather than JSON.)". You updated the fixtures to json.dumps (L109, L120) but not this sentence; it now documents the opposite of the code under test.

Non-blocking observations

• backend/routers/task_integrations.py L112/L278: the round-trip is correct — the only writer (get_oauth_url) stores json.dumps({'uid', 'app_key', 'created_at'}), all string values, and the callback's state_data.get('app_key') / ['uid'] access behaves identically on the JSON-loaded dict.
• Rollout note: states written before deploy hold Python-repr strings; consuming one now raises inside the try and returns None (invalid_state). Keys expire after OAUTH_STATE_EXPIRY = 600s, so the blip is bounded to in-flight flows within a ~10-minute window at deploy time. Fine — just worth knowing when this ships.
• The sibling validate_and_consume_oauth_state in routers/integrations.py additionally guards isinstance(loaded, dict) before returning; not strictly needed here since this writer only ever stores a dict, but it would be a cheap consistency win.

Requesting changes for the stray file and the stale docstring; the serialization swap itself looks good.

Automated review on behalf of the maintainers. Human sign-off specifically wanted on the OAuth state serialization change in the auth flow before merge.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[kodjima33]

Approve only: legitimate security hardening (ast.literal_eval->json) but PR Metadata Preflight CI check is failing — hard floor blocks merge.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[cursor]

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_2003998c-6a35-4ff2-adeb-6b7abd43cab8)

[undivisible]

Addressed Git-on-my-level CRs + Codex P2:

• deleted stray root pr_body.md
• Failure-Class: none in the PR body
• docstring now says json.loads / JSON fixtures
• in-flight pre-migration str(dict) OAuth state (10-min TTL) is still accepted by mapping single quotes to JSON, without bringing back ast.literal_eval

Pushed in f80686a.

Resolved on f80686a: the stray pr_body.md is gone (diff is now only task_integrations.py and its unit test), the stale ast.literal_eval docstring is corrected, and Failure-Class: none is declared (failure-class-protocol passes). No remaining change requests from this review.

[Git-on-my-level]

Thanks — all three items from the earlier change request are verified fixed on f80686a:

• pr_body.md is gone; the diff now touches only backend/routers/task_integrations.py and backend/tests/unit/test_task_integrations_oauth_atomic.py.
• The stale docstring in test_task_integrations_oauth_atomic.py (L6–7) now correctly says the handler parses with json.loads and the fixture stores JSON — matching the fixtures at L109/L120.
• Failure-Class: none is declared in the description, and failure-class-protocol passes on this head.

The serialization change itself reads right at the new head: get_oauth_url (L287) writes json.dumps(...), and validate_and_consume_oauth_state (L111–125) parses with json.loads plus a bounded fallback that maps pre-migration single-quote str(dict) payloads to JSON (fail-closed to None on anything unparseable, window bounded by the 10-minute OAUTH_STATE_EXPIRY). The new isinstance(state_data, dict) guard mirrors the sibling implementation in routers/integrations.py, and the atomic GETDEL consume path is untouched. Since the writer always stored string-only values with created_at already ISO-formatted at write time, the round-trip behavior for uid/app_key is unchanged.

One thing that is not yours: the failing PR Metadata Preflight job. It fails in firestore-query-coverage with "baseline registered_serving is 62, current inventory reports 73" — I reproduced the identical failure on unmodified main (the committed baseline is stale relative to recently merged query work; your head's coverage inventory is identical to main's). This diff adds or changes no Firestore query, so nothing here can turn it green; it should clear when the baseline regen lands (see #12480). The advisory "task_integrations.py is 905 lines" warning is likewise pre-existing.

Once that repo-side blocker clears, this just needs a fresh maintainer sign-off on the current head, since the earlier approval predates f80686a's fallback logic.

Automated review on behalf of the maintainers. Human sign-off specifically wanted on the current head's OAuth state parse-fallback in the auth flow before merge.

---

by AI on behalf of David — if you need David's attention urgently, please @Git-on-my-level and escalate with need human response.

[kodjima33]

Genuine CSRF/DoS-hardening fix replacing ast.literal_eval with json.dumps/loads for OAuth state stored in Redis, with backward-compat parsing for legacy single-quote values and updated tests; would score 5/5 on the confidence gate but 'PR Metadata Preflight' and 'Hygiene' required checks are actively failing, which trips the CI hard floor. Approve-only, held from merge on CI.