Skip to content

Fix MiMo Firefox session cookie import#1565

Open
aaronflorey wants to merge 8 commits into
steipete:mainfrom
aaronflorey:fix/mimo-firefox-cookies
Open

Fix MiMo Firefox session cookie import#1565
aaronflorey wants to merge 8 commits into
steipete:mainfrom
aaronflorey:fix/mimo-firefox-cookies

Conversation

@aaronflorey

@aaronflorey aaronflorey commented Jun 15, 2026

Copy link
Copy Markdown

Summary

  • recover complete MiMo authentication from Firefox's current session-cookie state when its persisted cookie database lacks usable authentication rows
  • follow Firefox's clean/recovery/recovery-backup/clean-backup/latest-upgrade order and stop at the first structurally valid state so older backups cannot resurrect stale credentials
  • decode only canonical bounded MOZLZ4 framing, parse only top-level cookie records, and accept only the unpartitioned default Firefox context
  • keep session and persisted sources independent: complete session authentication replaces persisted profile rows; partial session authentication never mixes across sources
  • add focused malformed-framing, ordering, source-isolation, origin/type, resource-limit, logout, and opaque-profile coverage

Root cause

Firefox keeps MiMo's active authentication values as session cookies. SweetCookieKit discovers the Firefox profile and reads persisted Gecko cookies, but active MiMo authentication may exist only in Firefox session restore. The original patch also treated partial restore records and older backups as mergeable sources, decoded ambiguous LZ4 payloads, recursively accepted nested cookie-shaped JSON, and discarded Firefox origin isolation.

Verification

Exact head: 28f780a87b85fad09982cedf7e0ebfe874e4e3dd

  • focused MiMo/Firefox tests: 53 passed
  • parser hardening suite: 7 passed
  • make check: passed; SwiftFormat clean and SwiftLint reported zero violations
  • make test: all 41 groups passed on the exact head
  • local autoreview: clean after the bounded-backup follow-up (0.88)
  • whole-branch autoreview: clean (0.86)
  • hosted exact-head CI: pending

Safety and proof boundary

  • only known MiMo cookie names on xiaomimimo.com are accepted; cookie values are never logged
  • every candidate is independently capped at 64 MiB input, 128 MiB declared decoded output, and 4,096 top-level cookies before filtering
  • malformed or unsafe candidates may fall back in Firefox's defined order; the first successfully decoded state remains authoritative even when empty or partial
  • non-default container, private-browsing, partition, unknown-origin, and wrong-typed origin attributes fail closed
  • Firefox sessionstore serializes session cookies only, so incomplete session authentication does not invalidate independent persisted authentication
  • SweetCookieKit's persisted Gecko rows do not expose Firefox origin attributes; this PR makes no container-aware persisted-cookie claim
  • merge remains blocked pending an authorized, redacted, latest-head Firefox recovery proof; no local account success is claimed

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

Copy link
Copy Markdown

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: ae2aec1db7

ℹ️ 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 Sources/CodexBarCore/Providers/MiMo/MiMoCookieImporter.swift Outdated
@clawsweeper

clawsweeper Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 12:27 PM ET / 16:27 UTC.

Summary
The branch adds Firefox sessionstore cookie recovery for MiMo, hardens MOZLZ4/session-cookie parsing, changes Firefox auth-source precedence, and adds focused importer/provider tests.

Reproducibility: yes. at source level, not by live run. Current main imports persisted SweetCookieKit rows only, so a Firefox profile whose MiMo auth exists only in sessionstore is not inspected.

Review metrics: 2 noteworthy metrics.

  • Changed surface: 5 files, +972/-2. The auth-cookie importer change is large enough that maintainers should review behavior and proof, not only CI status.
  • Test expansion: 2 test files, +573 lines. The PR adds substantial synthetic coverage for the new sessionstore parser and MiMo import path.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦞 diamond lobster
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Add redacted latest-head Firefox MiMo recovery proof from a real profile using terminal output, logs, screenshot, or recording while hiding private tokens and account details.
  • Update the PR body after adding proof so ClawSweeper re-reviews automatically, or ask a maintainer to comment @clawsweeper re-review if it does not.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Missing: the PR body reports tests/checks but explicitly says authorized redacted Firefox recovery proof is still pending and no local account success is claimed. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] No authorized redacted latest-head Firefox recovery proof is attached; the PR body says no local account success is claimed.
  • [P1] The patch changes how MiMo authentication cookies are discovered and selected for Firefox profiles, so live proof should cover session-only recovery and persisted-source isolation before merge.

Maintainer options:

  1. Require latest-head Firefox recovery proof (recommended)
    Ask for redacted terminal output, logs, screenshot, or recording from a real Firefox MiMo session showing session-only recovery without exposing private cookie values or account details.
  2. Accept test-only auth coverage
    Maintainers can intentionally merge based on the synthetic parser/provider tests and owner review, accepting that live Firefox account recovery was not demonstrated.
  3. Pause until authorized proof exists
    Keep the PR open or close it without merge if no authorized Firefox MiMo profile proof can be supplied for this sensitive auth path.

Next step before merge

  • [P1] Manual review/proof is needed because automation cannot produce the contributor's authorized Firefox MiMo session evidence.

Security
Cleared: The diff is auth-sensitive, but it bounds parsing, avoids logging cookie values, and introduces no concrete supply-chain or security-boundary regression.

Review details

Best possible solution:

Land this after maintainer-approved live Firefox proof shows complete session-only MiMo auth recovery and no cross-source credential mixing at the latest head.

Do we have a high-confidence way to reproduce the issue?

Yes at source level, not by live run. Current main imports persisted SweetCookieKit rows only, so a Firefox profile whose MiMo auth exists only in sessionstore is not inspected.

Is this the best way to solve the issue?

Yes, the bounded parser plus source-isolated replacement path looks like the narrow maintainable fix. The missing piece is real authorized Firefox proof, not an obvious code repair from this review.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 10dd11fb7e67.

Label changes

Label justifications:

  • P2: This is a normal-priority MiMo auth import fix with limited provider-specific blast radius.
  • merge-risk: 🚨 auth-provider: The PR changes how MiMo authentication cookies are discovered, selected, and prioritized for Firefox profiles.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦞 diamond lobster.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Missing: the PR body reports tests/checks but explicitly says authorized redacted Firefox recovery proof is still pending and no local account success is claimed. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: Recently changed browser cookie access isolation on current main and actively reviewed/updated this auth-sensitive MiMo Firefox path in the PR discussion. (role: recent area contributor; confidence: high; commits: 501cb51c210d, 28f780a87b85; files: Sources/CodexBarCore/BrowserCookieAccessGate.swift, Sources/CodexBarCore/Providers/MiMo/MiMoCookieImporter.swift)
  • Yuxin-Qiao: Introduced the merged multi-browser MiMo cookie import path and recently merged adjacent MiMo auth redirect fallback work. (role: feature owner; confidence: high; commits: 050b13925da4, 77044e418aa7; files: Sources/CodexBarCore/Providers/MiMo/MiMoCookieImporter.swift, Sources/CodexBarCore/Providers/MiMo/MiMoUsageFetcher.swift, Tests/CodexBarTests/MiMoProviderTests.swift)
  • debpramanik: Added the original Xiaomi MiMo provider and MiMo cookie importer that this PR extends. (role: introduced behavior; confidence: medium; commits: f9a2918afcf1; files: Sources/CodexBarCore/Providers/MiMo/MiMoCookieImporter.swift, Sources/CodexBarCore/Providers/MiMo/MiMoUsageFetcher.swift, Tests/CodexBarTests/MiMoProviderTests.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete steipete changed the title fix: import xiaomi mimo cookies from firefox properly Fix MiMo Firefox session cookie import Jun 19, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. labels Jun 19, 2026

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

Copy link
Copy Markdown

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: 924ccbb23d

ℹ️ 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 Sources/CodexBarCore/Providers/MiMo/MiMoCookieImporter.swift Outdated
@steipete steipete force-pushed the fix/mimo-firefox-cookies branch from 924ccbb to ead2aab Compare June 19, 2026 13:12

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

Copy link
Copy Markdown

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: ead2aab576

ℹ️ 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 Sources/CodexBarCore/Providers/MiMo/MiMoCookieImporter.swift Outdated
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 19, 2026
@steipete steipete force-pushed the fix/mimo-firefox-cookies branch from ead2aab to 28c8728 Compare June 19, 2026 15:53

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

Copy link
Copy Markdown

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: 28c8728bef

ℹ️ 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 Sources/CodexBarCore/Providers/MiMo/MiMoCookieImporter.swift Outdated
@clawsweeper clawsweeper Bot removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 19, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants