Skip to content

fix: retain normalized ZoomMate parent cookies - #2507

Closed
weddle wants to merge 2 commits into
steipete:mainfrom
weddle:fix/zoommate-parent-cookie-domain
Closed

fix: retain normalized ZoomMate parent cookies#2507
weddle wants to merge 2 commits into
steipete:mainfrom
weddle:fix/zoommate-parent-cookie-domain

Conversation

@weddle

@weddle weddle commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a live ZoomMate regression introduced by the host-partitioned cookie routing added during review of #2344.

SweetCookieKit normalizes Chromium parent-domain .zoom.us cookies to zoom.us before creating HTTPCookie values. The current partitioning logic interprets that bare value as host-only, dropping the shared Zoom SSO cookies for both approved API hosts and preventing any browser session from being imported.

This patch:

  • treats the known shared zoom.us parent domain as valid for both ai.zoom.us and zoommate.zoom.us;
  • keeps leaf-host cookies exact-host only;
  • adds regression coverage for the normalized parent-domain form.

Live reproduction

Using the same signed-in Chrome session:

Build Result
Pre-merge ZoomMate snapshot (979fd6c) Successfully fetched credits
Current build No ZoomMate session is cached and no session cookies were imported from Chrome

Validation

  • swift build -c debug --target CodexBarCore passes.
  • The focused test command is currently blocked by an unrelated existing compile error in ClaudeCLIBackgroundAvailabilityTests (missing includePrepaidBalance argument).

@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: a550713013

ℹ️ 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 on lines +126 to +127
if normalizedDomain == Self.sharedParentCookieDomain {
return normalizedHost.hasSuffix("." + normalizedDomain)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve host-only scope for bare zoom.us cookies

When a browser profile contains a host-only cookie set on zoom.us, this unconditional special case treats it as a parent-domain cookie and adds it to requests for both API subdomains, even though the browser would send it to neither. Because the import query deliberately includes zoom.us, such records can enter this path and may leak or duplicate cookie names across hosts; distinguish domain cookies using the original BrowserCookieRecord.domain before makeHTTPCookies erases that information rather than treating every bare zoom.us value as shared.

Useful? React with 👍 / 👎.

@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. P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jul 29, 2026
@clawsweeper

clawsweeper Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 31, 2026, 7:02 PM ET / 23:02 UTC.

ClawSweeper review

What this changes

The PR restores Chromium’s domain-versus-host-only cookie scope before ZoomMate builds headers for its two approved API hosts, so normalized shared zoom.us cookies import without forwarding leaf-host cookies to a sibling host.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

This PR is still necessary: current main does not contain the repair, and current source loses normalized parent-domain cookies before either approved ZoomMate API host receives them. The proposed metadata recovery is narrow, fail-closed, and backed by redacted packaged-CLI proof; likely related people are Peter Steinberger for the current host-partitioning behavior and Ryan Weddle for the ZoomMate provider and repair history.

Priority: P1
Reviewed head: 4acee6b2884fd8c4267317c443fa8ff0d5cb81ec

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) Strong redacted live proof and focused coverage support a narrow, correct repair with bounded compatibility and cookie-routing risk.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The PR includes a redacted after-fix packaged CLI run against a signed-in Chrome session that successfully imports cookies, completes the ZoomMate login bootstrap, and retrieves credits.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR includes a redacted after-fix packaged CLI run against a signed-in Chrome session that successfully imports cookies, completes the ZoomMate login bootstrap, and retrieves credits.
Evidence reviewed 6 items Current regression path: Current main queries zoom.us alongside the two leaf domains, converts records to HTTPCookie, and then treats a bare normalized domain as host-only; this drops a normalized parent-domain cookie for both allowed leaf hosts.
Narrow fail-closed repair: The PR joins decrypted records to raw Chromium host_key metadata by normalized domain, name, and path; it drops records with missing, ambiguous, or partitioned scope metadata before building headers.
Host allowlist remains enforced: The repaired sender accepts only the two existing ZoomMate hosts, requires root-path cookies, and applies exact matching for host-only cookies or dot-boundary suffix matching for domain cookies.
Findings None None.
Security None None.

How this fits together

CodexBar reads cookies from a user-selected browser profile for enabled providers, then creates request headers for that provider’s fixed endpoints. ZoomMate uses the resulting per-host headers to bootstrap a session and fetch usage and credits.

flowchart LR
  Browser[Chrome Cookies database] --> Snapshot[Copied cookie snapshot]
  Snapshot --> Records[Decrypted cookie records]
  Snapshot --> Metadata[Chromium scope metadata]
  Records --> Resolver[Fail-closed scope resolver]
  Metadata --> Resolver
  Resolver --> Headers[Approved-host cookie headers]
  Headers --> ZoomMate[ZoomMate login and usage requests]
Loading

Before merge

  • Resolve merge risk (P1) - The repair intentionally fails closed when Chromium scope metadata is missing, ambiguous, or partitioned; affected browser profiles will surface the existing no-session recovery path rather than forwarding uncertain credential-bearing cookies.
  • Resolve merge risk (P1) - The new SQLite metadata read is part of the browser-cookie trust boundary, so the fixed-host allowlist and fail-closed tests should remain intact through future provider changes.
  • Complete next step (P2) - The contributor has provided adequate live proof and the diff has no discrete repair finding; the remaining action is a maintainer’s merge review of the credential-routing boundary.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 5 files affected; 422 added, 61 removed The change adds one macOS-only scope reader and replaces two obsolete scope tests with three focused regression tests.
Validation coverage 3 new scope tests; 2 macOS test shards passed The tests directly cover the previously unsafe normalized parent-domain distinction and the expected fail-closed cases.

Merge-risk options

Maintainer options:

  1. Merge with the fail-closed boundary intact (recommended)
    Accept the narrow compatibility repair after confirming that all uncertain Chromium scope records remain excluded and headers remain limited to ai.zoom.us and zoommate.zoom.us.
  2. Pause for additional profile coverage
    Request a targeted fixture or manual check only if maintainers need evidence for a supported Chromium schema not represented by the existing scope tests.

Technical review

Best possible solution:

Merge the focused scope-recovery fix after maintainer review, preserving the two-host allowlist and the fail-closed treatment of uncertain Chromium cookie metadata.

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

Yes. Current main’s normalized bare-domain handling is visible in the importer, and the contributor supplies a signed-in Chrome before/after packaged-CLI reproduction; this read-only review did not access any real browser cookie store.

Is this the best way to solve the issue?

Yes. Recovering Chromium’s original scope metadata before the existing fixed-host header filter is the narrowest safe repair; treating every normalized zoom.us cookie as shared would incorrectly forward host-only cookies.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8ef86077e70a.

Labels

Label justifications:

  • P1: Current main can fail to import a valid signed-in ZoomMate Chrome session, blocking provider usage retrieval for affected users.
  • merge-risk: 🚨 compatibility: The import path changes how existing browser cookie profiles are classified and may deliberately reject uncertain profile records.
  • merge-risk: 🚨 security-boundary: The code determines which credential-bearing browser cookies can be attached to ZoomMate’s approved network hosts.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR includes a redacted after-fix packaged CLI run against a signed-in Chrome session that successfully imports cookies, completes the ZoomMate login bootstrap, and retrieves credits.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR includes a redacted after-fix packaged CLI run against a signed-in Chrome session that successfully imports cookies, completes the ZoomMate login bootstrap, and retrieves credits.

Evidence

What I checked:

Likely related people:

  • Peter Steinberger: Authored the current-main commit that introduced the ZoomMate host-partitioned cookie behavior this PR repairs. (role: recent area contributor; confidence: high; commits: f745fc310f84; files: Sources/CodexBarCore/Providers/ZoomMate/ZoomMateCookieImporter.swift)
  • Ryan Weddle: Authored the earlier ZoomMate importer history and the proposed scope-recovery repair, providing direct domain knowledge of the provider flow. (role: original provider contributor and repair author; confidence: high; commits: 979fd6c4036e, 4acee6b2884f; files: Sources/CodexBarCore/Providers/ZoomMate/ZoomMateCookieImporter.swift, Sources/CodexBarCore/Providers/ZoomMate/ZoomMateChromiumCookieScopeReader.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Maintainer review should confirm the fail-closed metadata and fixed-host boundary before merge.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (4 earlier review cycles)
  • reviewed 2026-07-29T18:08:14.916Z sha a550713 :: needs real behavior proof before merge. :: [P2] Preserve host-only scope for bare zoom.us cookies
  • reviewed 2026-07-29T22:52:19.254Z sha 4acee6b :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-30T01:07:29.498Z sha 4acee6b :: needs maintainer review before merge. :: none
  • reviewed 2026-07-31T16:25:23.786Z sha 4acee6b :: needs maintainer review before merge. :: none

@weddle

weddle commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Live verification for 4acee6b2

I verified this revision against a signed-in ZoomMate account using Chrome and
the packaged CodexBar CLI.

Browser behavior

Chrome DevTools Protocol reported the following routing decisions for the real
request destinations:

Chromium storage scope ai.zoom.us zoommate.zoom.us
.zoom.us domain cookie sent sent
host-only zoom.us cookie DomainMismatch DomainMismatch
.ai.zoom.us domain cookie sent DomainMismatch
host-only ai.zoom.us cookie sent DomainMismatch

A controlled replay to the login endpoint also showed that the .zoom.us
parent-domain group returned a NAK, while the AI-leaf group alone did not. This
confirms that the parent-domain cookies are material to the login bootstrap.

SweetCookieKit normalizes both .zoom.us and host-only zoom.us to zoom.us,
so the normalized domain cannot reproduce Chrome's routing by itself. This
revision reads the non-secret Chromium scope metadata from the same copied
database snapshot used for decryption, joins it to the decrypted records by
domain/name/path, and then applies RFC 6265 matching:

  • host-only cookies require an exact host match;
  • domain cookies allow the exact domain or a dot-boundary suffix;
  • missing, ambiguous, or partitioned scope metadata fails closed;
  • the final headers remain restricted to the two fixed ZoomMate request hosts.

Automated verification

The focused suite passes:

$ swift test --filter ZoomMateCookieScopeTests
3 tests passed

It covers parent-domain routing, host-only and leaf isolation, dot-boundary
matching, hostile suffix rejection, root-path enforcement, and fail-closed
handling for missing/ambiguous/partitioned metadata.

Current PR checks:

The remaining Linux ARM64
failure

is in the unrelated CLICardsClaudeSwapTests fake-executable test
(NSCocoaErrorDomain error 256). The CLI release build succeeded on ARM64, and
the identical test passed on Linux x64. The implementation and focused tests in
this revision are macOS-gated.

Packaged live CLI verification

$ CodexBar.app/Contents/Helpers/CodexBarCLI usage \
    --provider zoommate --source web --format json

Sanitized result:

exitCode: 0
provider: zoommate
source: web
loginMethod: Cookie
primary credits payload: present

The packaged bundle identified commit 4acee6b2, and
codesign --verify --deep --strict passed. This confirms that the revised
importer can read the live Chrome session, complete the ZoomMate login
bootstrap, and fetch current credits.

For redaction: this comment includes no account identifier, usage amount,
cookie name or value, cookie header, bearer/NAK value, authorization header,
response body, or credential. The underlying CDP capture retained only routing
metadata (including cookie names and scopes); it never emitted or saved cookie
values, token values, authorization headers, or response bodies.

@weddle

weddle commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 30, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed 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. labels Jul 30, 2026
@steipete

steipete commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Confirmed @weddle's diagnosis and followed the single cookie-access-path policy.

The exact loss happens between SweetCookieKit's Chromium reader and ZoomMate's destination partitioning. Chromium stores the shared SSO cookies (including the _zm_* family and cf_clearance) with host_key = .zoom.us. SweetCookieKit 0.5.1 normalizes that to BrowserCookieRecord.domain = zoom.us, and HTTPCookie also exposes the bare form. ZoomMate then interprets a bare domain as host-only, so those shared cookies are dropped for both ai.zoom.us and zoommate.zoom.us. A genuinely host-only zoom.us cookie becomes indistinguishable, so treating every normalized zoom.us record as shared would cross the cookie security boundary.

SweetCookieKit 0.5.1 does not retain or expose this scope. Its schema-v24 work validates and strips the encrypted value's domain hash, but BrowserCookieRecord still loses the leading-dot distinction.

I opened steipete/SweetCookieKit#19 with the library-level fix. It adds explicit host-only/domain scope to normalized records, carries it through Safari, Chromium, and Gecko imports, and includes Chromium fixtures for .zoom.us versus host-only zoom.us. The change is commit steipete/SweetCookieKit@7342f04 and credits Ryan's diagnosis/co-authorship.

Validation is green: SweetCookieKit focused tests, its full 69-test suite, strict lint, and autoreview; CodexBar make check, all focused ZoomMate suites, and make test also pass on current main.

What remains: merge and release SweetCookieKit (no tag/release was made here), bump CodexBar to that release, then update ZoomMate to use BrowserCookieRecord.scope for exact-host versus RFC domain matching and add the provider-level regression fixture. I am leaving #2507 open for that follow-through.

@steipete

steipete commented Aug 3, 2026

Copy link
Copy Markdown
Owner

The library-path fix is up in #2627. It bumps SweetCookieKit to 0.5.2 and uses the preserved browser cookie scope so parent .zoom.us cookies reach both ZoomMate API hosts without leaking host-only cookies. This issue will be closed when the PR merges.

@steipete

steipete commented Aug 3, 2026

Copy link
Copy Markdown
Owner

Fixed on main via the library path: SweetCookieKit 0.5.2 now preserves host-only vs domain scope on normalized records (SweetCookieKit#19), and #2627 bumps CodexBar and makes ZoomMate route by real scope — parent .zoom.us domain cookies reach both API hosts, host-only cookies stay host-only, with regression fixtures reproducing exactly the scenario you diagnosed.

Your diagnosis was precisely right; the only change from this PR's approach is where the fix lives — scope belongs in the shared cookie library rather than a provider-local Chromium reader, so every cookie provider benefits. Changelog credits you. Closing in favor of the landed pair; ships in 0.47.1. Thanks @weddle!

@steipete steipete closed this Aug 3, 2026
@weddle
weddle deleted the fix/zoommate-parent-cookie-domain branch August 9, 2026 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P1 Urgent regression or broken agent/channel workflow affecting real users now. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants