feat(zoom): add Zoom API client and transcript parsing - #14481
feat(zoom): add Zoom API client and transcript parsing#14481Subash-Mohan wants to merge 8 commits into
Conversation
Groundwork for the Zoom connector: a Server-to-Server OAuth client that owns every call to Zoom, and a WebVTT parser for the transcripts it returns. Nothing is registered as a connector yet, so this changes no indexing behaviour — the connector itself follows separately. The client covers the endpoints the first connector needs: fetching a Cloud Recording transcript, reading an occurrence's details, and listing a meeting's past occurrences. A recurring meeting records each run separately, and passing the bare meeting id to the transcript endpoint only ever reaches the latest one, which is why the occurrence listing exists. Meeting UUIDs need care in a URL: they can contain "/", and Zoom's docs require encoding twice when one starts with "/" or contains "//", because something in front of their API decodes a layer first. The VTT parser follows the W3C spec rather than matching on shape. Zoom documents that the file is VTT and carries timestamped sections, but never defines the layout inside it. Parsing by shape drops real speech: a cue containing "-->" reads as a timing line, and a cue that is only a number reads as a cue index. Both are covered by tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBNe54dXPELTXXGdk1uYR6
|
Full-stack Preview (frontend + backend)
|
🖼️ Visual Regression Report
|
Greptile SummaryAdds the foundation for a Zoom connector without registering an indexing connector.
Confidence Score: 5/5The PR appears safe to merge; no outstanding or newly introduced actionable failures remain. The latest revision correctly classifies token endpoint authorization failures with connector-validation exceptions. All previous findings were manually resolved, and the current code contains their corresponding fixes. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[ZoomClient] --> B[Zoom OAuth token endpoint]
A --> C[Zoom meetings API]
C --> D[Transcript metadata]
D --> E{Downloadable?}
E -->|Yes| F[Validated Zoom download URL]
F --> G[SSRF-safe redirect handling]
G --> H[WebVTT parser]
H --> I[Indexable transcript text]
E -->|No| J[Skip transcript]
Reviews (4): Last reviewed commit: "test(zoom): run the real guard in the do..." | Re-trigger Greptile |
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Zoom can reject an access token before the expiry it handed us, so a 401 on a normal request did not mean the credentials were bad. Reporting it as CredentialExpiredError cancelled the whole indexing attempt, and five of those in a row mark the connector invalid and email the admins — over credentials that were fine. A 401 now clears the cached token and retries once, and only a second rejection is reported. The transcript download went straight to requests.get, so it had no retry and surfaced a bare HTTPError instead of the typed errors every other call maps. Both paths now share one helper, so they cannot drift apart again. Retrying is safe here because a 401 is refused before the request does anything, so there is no side effect to repeat. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBNe54dXPELTXXGdk1uYR6
download_transcript_vtt took any URL and attached the account-wide bearer token to it, so a wrong or tampered download URL would hand the credential to whatever host it named, or reach an internal service. Zoom supplies the URL, but that is not a reason to send a credential somewhere unchecked. The URL is now rejected unless it is a zoom.us host, and it also goes through the repository's outbound URL check so a Zoom subdomain pointing at an internal address is refused too. Only the first host needs checking, because requests drops the Authorization header on a cross-host redirect — which matters, since Zoom's downloads legitimately redirect to a CDN. The allowlist matches the API base URL this client already hardcodes, so it adds no limitation. A test ties the two together: Zoom for Government lives on api.zoomgov.com, and pointing the client there without moving the allowlist would fail every download. A rejected URL raises ValueError rather than a validation error, so one bad URL fails its own document instead of cancelling the indexing run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBNe54dXPELTXXGdk1uYR6
WebVTT forbids a literal "&" or "<" in cue text, so a speaker saying "R&D" reaches us as "R&D". The parser stripped markup but never decoded, so that went into the index verbatim and nobody searching for "R&D" would find the meeting. Decoding happens after markup is stripped, not before. An escaped "<v Jane>" is something a speaker actually said; decoding first would turn it into a tag and delete it. A decoded non-breaking space becomes a normal one, matching what the Fireflies connector already does, because it is invisible but will not match a space someone types into search. Also moves the module's two shape-matching gotchas into a header rather than repeating them beside each regex. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBNe54dXPELTXXGdk1uYR6
Transcript downloads bypassed the session and ran with no retries, so one
transient 429 lost a transcript that every other call would have retried.
Mount the retry adapter on the scheme, not per URL: the API, the token
endpoint and the download are three different Zoom hosts, and a host
without its own mount falls back to the no-retry default.
Readiness came from download_url alone. Zoom documents can_download,
download_url and download_restriction_reason as mutually exclusive, but
its own example returns all three together. Require all three to agree.
Keep meeting_topic, host_id and can_download from the transcript
response. meeting_topic is the only field the connector needs from
GET /past_meetings/{id}, which is the one call that Zoom limits to
meetings less than one year old.
Report Zoom's error code and message. The requests library stops at
"400 Client Error" and drops the body, which hides codes such as 12702.
Remove the next_page_token check. The endpoint defines three fields and
no paging token, and the truncation that does occur is Zoom's 15-month
limit, which no check can detect.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EBNe54dXPELTXXGdk1uYR6
|
@Subash-Mohan I have started the AI code review. It will take a few minutes to complete. |
Go coverage
|
| Package | Coverage | Floor | Change |
|---|---|---|---|
| internal/portutil | 91.7% | 75.0% | +16.7 |
| total | 36.5% | 36.4% | +0.1 |
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
The download URL was checked once, then requests followed the redirects it returned without any further check. An open redirect on a Zoom host made the connector fetch a private address and index the body as a transcript. Dropping the Authorization header on a cross-host redirect stops the token leaking, but it does not stop the request. Stop following redirects automatically and send each hop through ssrf_safe_get, which resolves and validates the address and pins it against DNS rebinding. Do not pass the token to ssrf_safe_get: it forwards the same headers to every hop, so the account-wide credential would reach each redirect target. The first request keeps the session, and with it the retry policy, because ssrf_safe_get makes a new session per hop and does not retry. The storage host authenticates from the signed URL. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBNe54dXPELTXXGdk1uYR6
|
@Subash-Mohan I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 7 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
A 403 from Zoom's token endpoint raised a plain HTTPError, so a permanent misconfiguration such as a deactivated app or a scope that was never granted looked transient. Onyx retried it forever and never told the admin. Map it to InsufficientPermissionsError, which is what the API path already does. Raise CredentialInvalidError instead of CredentialExpiredError on a 401. A Server-to-Server client secret has no expiry, so "expired" sends admins to look for a renewal that does not exist. Both types are terminal, so only the message an admin reads changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBNe54dXPELTXXGdk1uYR6
The test re-implemented the host check in its assertion instead of calling it. It caught a change to _API_BASE_URL, but gutting _reject_non_zoom_download_url left it passing, because the assertion verified the two constants agreed under the test's own copy of the matching rule rather than the guard's. Call the guard with a URL built from the API host, so both sides run and neither is duplicated. The test now fails when _API_BASE_URL moves away from the allowlist and when the guard's own predicate changes. _ZOOM_HOST is no longer imported, since the test no longer repeats the rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBNe54dXPELTXXGdk1uYR6
|
@Subash-Mohan I have started the AI code review. It will take a few minutes to complete. |
Description
How Has This Been Tested?
Unit tests added for the client and the parser (32 tests).
Additional Options
Summary by cubic
Adds a Zoom API client and WebVTT transcript parser as the foundation for the Zoom connector. No connector is registered yet, so indexing behavior is unchanged; the connector that uses this lands in a follow-up PR.
New Features
can_download,download_url, anddownload_restriction_reasonagree.Written for commit 8f561d6. Summary will update on new commits.