feat(gmail): support custom API endpoint - #1003
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs maintainer review before merge. Reviewed August 18, 2026, 3:09 PM ET / 19:09 UTC. ClawSweeper reviewWhat this changesThe PR adds Merge readinessKeep open for explicit maintainer approval: the implementation is sound, but it deliberately creates an opt-in boundary that sends OAuth-authenticated Gmail traffic to a configured endpoint. Priority: P1 Review scores
Verification
How this fits togetherGmail commands obtain authenticated Google API services from the shared runtime factory. This setting changes the destination for those Gmail requests while leaving the default Google endpoint unchanged when unset. flowchart LR
A[Operator environment] --> B[Endpoint validation]
B --> C[Runtime service setup]
C --> D[Gmail service factory]
D --> E[Authenticated Gmail commands]
E --> F[Configured Gmail API endpoint]
Decision needed
Why: The code safely constrains transport and documents the consequence, but whether this trust boundary belongs in the supported product is a maintainer security and product decision. Before merge
Findings
Agent review detailsSecurityNeeds attention: The patch deliberately adds a credential-routing security boundary that needs maintainer acceptance, despite its HTTPS-or-loopback validation. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: If a maintainer sponsors this opt-in proxy capability, retain the default Google endpoint, the HTTPS-or-loopback restriction, and the explicit credential-routing documentation. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: this is new opt-in behavior, and the PR supplies a live loopback run covering both authenticated request paths. Is this the best way to solve the issue? Unclear: the implementation is narrow and validated, but only a maintainer can determine whether this credential-routing capability is an acceptable supported boundary. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against eb85a99366eb. LabelsLabel justifications:
EvidenceSecurity concerns:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (4 earlier review cycles)
|
Signed-off-by: sallyom <somalley@redhat.com>
91d4451 to
a94dd84
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
7d8fcc9 to
5699ae2
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
wrt [P1]: |
|
Closing on security grounds rather than on effort — thank you for the work, but this can't land in its current shape. The webhook/forwarding path sends Gmail OAuth bearer credentials and mailbox contents to operator-supplied endpoints, and the OAuth transport reattaches those credentials across redirects. That means a misconfigured or attacker-influenced target URL — or a redirect chain to one — receives live Google credentials and message contents, not just the intended payload. That's a credential-exfiltration shape, so it fails closed for us regardless of how the feature is configured. If you want to pursue this, the design that would be reviewable: never attach the OAuth transport to outbound third-party requests (use a plain client with no credential provider), refuse to follow cross-origin redirects on that path entirely, and send only an explicitly constructed payload with no raw credential material. That plus live proof against a redirecting endpoint would make it a real conversation. |
Adds
GOG_GMAIL_BASE_URLto route Gmail API requests through a compatible custom endpoint, such as a security proxy or test server.The override applies to standard Gmail and batch-delete clients. Existing behavior is unchanged when the variable is unset.
Testing
go test ./internal/googleapimake cibin/gogand ran both command paths against a live loopback-compatible endpoint using a synthetic token and synthetic message identifiers. The endpoint logged only the request method/path and whether bearer authentication was present:No token value, mailbox content, or external Gmail account data was included in the proof.