Skip to content

feat(connectors): OAuth forward-out to sidecars (V2-14)#2203

Merged
kovtcharov merged 6 commits into
mainfrom
feat/oauth-forward-out-2154
Jul 18, 2026
Merged

feat(connectors): OAuth forward-out to sidecars (V2-14)#2203
kovtcharov merged 6 commits into
mainfrom
feat/oauth-forward-out-2154

Conversation

@kovtcharov-amd

Copy link
Copy Markdown
Collaborator

Before: a sidecar that needed a mailbox connection (e.g. the email agent needing the granted Gmail token) read the machine keyring/grants store directly and would have to hold long-lived OAuth refresh tokens itself. N sidecars each holding a refresh token rotate each other out, and a sidecar is the wrong place to custody a long-lived secret.

After: the daemon — the custody home — owns the refresh token and forwards only short-lived access tokens OUT to a new sidecar /v1/connections intake, per agent, honoring the grant model (only connectors granted to that agent are forwarded). The sidecar never receives the refresh token or the OAuth client secret; the daemon forwards on spawn, re-forwards on expiry, and withdraws on revocation/uninstall. No grant / expired token / forward failure is ever a silent empty credential — each surfaces a loud, actionable error where it's fixable.

Scoped to first-party connector forwarding (Google/Microsoft for the email agent) per the plan's §0.24 note that D3's third-party trust root is still open. Standalone-integrator mode (a sidecar with its own OAuth) is unchanged — forwarding only engages when the daemon sets the private forwarded-mode env channel on spawn.

Threads a reviewer evaluates independently

  • Core refresh reuseget_access_token_with_expiry(_sync) applies the same per-agent grant + OAuth-scope gate as get_access_token and additionally returns wall-clock expiry, reusing the existing client-neutral refresh engine (no duplicated refresh logic).
  • Daemon forwarderConnectionForwarder mints granted tokens and delivers them to the sidecar; forward-on-spawn via a new SidecarRegistry on_started hook; a mint failure (revoked/absent connection) withdraws any stale forward so the sidecar loses access loudly; /daemon/v1/agents/{id}/connections control plane maps an ungranted forward to 403 at the HTTP boundary.
  • Sidecar — in-memory forwarded-credentials store + resolver (loud on missing/expired/scope-short; never a keyring fallback in forwarded mode) and /v1/connections intake routes, added as additive contract 2.5; the four token resolvers delegate through the resolver so standalone mode is untouched.

Dependency / rebase note

Built in parallel with #2153 (daemon /host/v1 custody API, V2-12). #2153's route table (rag/memory/sessions/audit) does not cover connector forwarding, so forward-out is implemented as a daemon→sidecar push and does not consume #2153's surface — but it lands in the same src/gaia/daemon/ area. Expect to rebase onto #2153's PR once it merges. Draft until then.

Test plan

  • pytest tests/unit/test_connectors_forward_token.py — grant/scope gate + expiry (4)
  • pytest tests/unit/test_daemon_forward.py — forwarder + HTTP boundary: ungranted→403, granted→forwarded, token scoping, mint-failure withdraw, delivery→502, token-required (16)
  • pytest hub/agents/python/email/tests/test_forwarded_credentials.py — store/resolver + intake 401 boundary, metadata-only, standalone-vs-forwarded (17)
  • pytest hub/agents/python/email/tests/test_rest_contract.py — openapi regenerated; contract 2.5; new routes documented (1 pre-existing failure: test_agent_version_matches_package_metadata, stale editable-install metadata 0.1.0 vs source 0.5.0 — unrelated to this change)
  • black/isort/flake8 clean on changed src/ + tests/ files (project config)
  • Rebase onto feat(daemon): /host/v1 custody API v1 with per-agent scoping (V2-12) #2153 and re-run daemon suite
  • Agent UI daemon end-to-end: keyring removed, sidecar answers a mailbox call on a forwarded token; revoke → loud failure

Note: 6 pre-existing os.killpg/group-kill failures on Windows in the daemon manager/reap suites are environment-only (POSIX-only paths) and predate this change.

Part of #2014
Closes #2154

Before: a sidecar that needed a mailbox connection read the machine keyring
and grants store directly and would have to hold long-lived OAuth refresh
tokens itself — N sidecars each holding a refresh token rotate each other out,
and a sidecar is the wrong place to custody a long-lived secret.

After: the daemon (the custody home) owns the refresh token and forwards only
short-lived access tokens OUT to the sidecar's new /v1/connections intake, per
agent, honoring the grant model (only connectors granted to that agent are
forwarded). The sidecar never receives the refresh token or the OAuth client
secret; the daemon re-forwards on expiry and withdraws on revocation/uninstall.

Core (gaia.connectors): get_access_token_with_expiry(_sync) — the same per-agent
grant + OAuth-scope gate as get_access_token, additionally returning the token's
wall-clock expiry so the daemon knows when to re-forward. Reuses the existing
client-neutral refresh engine; no duplicated refresh logic.

Daemon (gaia.daemon): ConnectionForwarder mints granted connector tokens and
POSTs them to the sidecar; forward-on-spawn via a new registry on_started hook;
withdraw on mint failure (revocation) so the sidecar loses access loudly;
/daemon/v1/agents/{id}/connections control plane (ungranted forward → 403).
The manager sets a private forwarded-mode env var on spawn from the spec.

Sidecar (gaia_agent_email): in-memory forwarded-credentials store + resolver
(loud on missing/expired/scope-short — never a silent empty token, never a
keyring fallback in forwarded mode); /v1/connections intake routes (contract
2.5, additive); the four token resolvers delegate through the resolver so
standalone-integrator mode keeps working unchanged.

No silent fallbacks: no grant / expired / forward failure surfaces a loud,
actionable error at the point it is fixable.

Part of #2014
Closes #2154
@github-actions github-actions Bot added tests Test changes agent::email Email agent changes labels Jul 18, 2026
# Conflicts:
#	src/gaia/daemon/sidecars/manager.py
#	src/gaia/daemon/sidecars/spec.py
@kovtcharov
kovtcharov marked this pull request as ready for review July 18, 2026 05:36
# Conflicts:
#	src/gaia/daemon/app.py
#	src/gaia/daemon/server.py
@github-actions

Copy link
Copy Markdown
Contributor

Verdict: Approve with suggestions (draft PR — no blocking correctness or security issue).

This lands OAuth "forward-out": the daemon keeps the long-lived refresh token and pushes only short-lived access tokens to the email sidecar, per-agent and grant-gated. The design is clean and the security posture is careful — the refresh token and client secret never cross the process boundary, the forwarded list is metadata-only, and both are covered by tests. Everything fails loudly; nothing silently falls back to the keyring in forwarded mode.

One thing worth confirming before this leaves draft: the description says the daemon "re-forwards on expiry," but this PR only forwards on spawn. There's no timer or trigger in the diff that re-mints when a forwarded token nears expiry — so once the ~1h access token lapses, the sidecar raises a loud "expired, retry in a moment" that won't actually self-heal unless something (Agent UI? a follow-up?) calls the forward control plane. Worth clarifying where that re-forward gets driven, and softening the "retry in a moment" wording if recovery isn't automatic.

Two small polish items: one error message has a broken f-string so it prints a literal {provider}, and the daemon's forward_all route doesn't map a missing-grant_agent_id config error to a clean status (500 instead of 4xx). Details below.

🔍 Technical details

🟡 Important

Re-forward-on-expiry has no trigger in this PR (src/gaia/daemon/server.py:190, forward.py) — the only forward path wired is the on_started on-spawn push. The /daemon/v1/agents/{id}/connections/*/forward routes exist but nothing in the tree calls them on token expiry (grep-confirmed: no in-repo caller outside the new files). Steady state for a long-lived sidecar: forwarded token expires (~1h) → get_forwarded_token raises → mailbox calls fail until the sidecar restarts or an external actor re-forwards. Not a silent bug (it's loud), but it doesn't match the description's "re-forwards on expiry," and the user-facing message (forwarded_credentials.py:176) says "Retry in a moment," implying a transience the code doesn't provide. Please confirm the re-forward trigger lives in the Agent UI / a follow-up, and reword the message if recovery isn't automatic.

🟢 Minor

Broken f-string in the expired-token message (hub/agents/python/email/gaia_agent_email/forwarded_credentials.py:177) — the first segment is an f-string but the trailing one isn't, so {provider} renders literally.

    if (cred.expires_at - time.time()) < _EXPIRY_BUFFER_SECONDS:
        raise ConnectorsError(
            f"the forwarded '{provider}' access token has expired and the daemon "
            "has not re-forwarded a fresh one yet. Retry in a moment; if it "
            f"persists, reconnect '{provider}' in Settings → Connections."
        )

forward_all route leaks a config error as 500 (src/gaia/daemon/connections_routes.py:764)forward_all can raise NotGrantedError from _grant_agent_id when a spec has no grant_agent_id, but the route only catches UnknownAgentError, so a misconfigured spec surfaces as an opaque 500 rather than the 403 the /{provider}/forward route already uses for the ungranted case. The email spec sets grant_agent_id, so this only bites a future/misconfigured agent — cheap to catch here for consistency.

Edge: a token with no cached expiry can't be forwarded (forward.py:_post_forward)get_token_with_expiry documents a 0.0 fallback when the cache has no expiry, but the sidecar intake requires expires_at > 0 (ForwardedConnectionRequest), so such a forward is rejected as a 502. Narrow — get_or_refresh normally repopulates expiry on mint — and it fails loudly, so noting only in case you want the daemon to treat a 0.0 expiry as its own actionable error rather than a delivery failure.

Strengths

  • Security posture is the highlight — no refresh token or client secret ever forwarded, list is metadata-only, token never echoed, and every one of those invariants has a dedicated test (test_list_forwarded_is_metadata_only_never_token, the refresh_token/client_secret absence asserts in test_daemon_forward.py).
  • Clean seam extraction_authorize_access gives the grant+scope gate a single home shared by get_access_token and the new _with_expiry accessor, so the two can't drift; the injectable seams on ConnectionForwarder make the HTTP boundary genuinely unit-testable without a live keyring or sidecar.
  • Bundled docs updated in sync (README/SPEC/SKILL/CHANGELOG + regenerated openapi + contract 2.5), matching the CLAUDE.md rule that a contract change touches every doc that describes it.

# Conflicts:
#	src/gaia/daemon/server.py
#	src/gaia/daemon/sidecars/manager.py
#	src/gaia/daemon/sidecars/registry.py
@github-actions

Copy link
Copy Markdown
Contributor

🟡 The POST /daemon/v1/agents/{id}/connections/forward route is missing a NotGrantedError handler. forwarder.forward_all raises NotGrantedError (via _grant_agent_id) when called for an agent spec that has no grant_agent_id configured — but unlike forward_provider on the same router, forward_all lets it escape as an unhandled exception, producing a 500 with internal error detail in the body instead of the 403 the module docstring promises.

🔍 Technical details

src/gaia/daemon/connections_routes.py:48-56 — the forward_all handler catches only UnknownAgentError, while the sibling forward_provider handler on line 71-72 also catches NotGrantedError → 403. The module docstring at line 13-14 explicitly documents "provider not granted to the agent → 403", so this omission contradicts the spec.

src/gaia/daemon/forward.py:2056forward_all calls self._grant_agent_id(spec) before the per-provider loop; that helper raises NotGrantedError when spec.grant_agent_id is falsy.

The built-in email spec always has grant_agent_id set, so this only fires for a misconfigured or future agent spec. Still, the missing handler leaks the internal NotGrantedError message in a 500 body, and tests/unit/test_daemon_forward.py has no test for this path through the forward_all route.

Fix: add the same except NotGrantedError as e: raise HTTPException(status_code=403, ...) clause to forward_all that forward_provider already has.

@github-actions

Copy link
Copy Markdown
Contributor

🟡 The POST /daemon/v1/agents/{id}/connections/{provider}/forward route returns an opaque 500 when the OAuth connection is revoked or not connected, rather than a meaningful error. The module docstring promises distinct HTTP statuses for each failure class, but AuthRequiredError (raised by the mint when the connection is absent, revoked, or scope-short) is not caught by the route and escapes as {"detail":"Internal Server Error"} with no actionable context for the caller.

🔍 Technical details

src/gaia/daemon/connections_routes.py:58–74 — the forward_provider route catches UnknownAgentError (404), NotGrantedError (403), and ForwardDeliveryError (502), but ConnectionForwarder.forward_provider in src/gaia/daemon/forward.py:2005–2028 re-raises the mint exception after logging and withdrawing any stale forward. When the mint fails with AuthRequiredError.Reason.NOT_CONNECTED or CONNECTION_MISSING_SCOPES, that exception propagates through run_in_threadpool and FastAPI converts it to 500.

Suggested fix — add a catch for AuthRequiredError before the ForwardDeliveryError catch:

from gaia.connectors.errors import AuthRequiredError
...
except AuthRequiredError as e:
    raise HTTPException(status_code=502, detail=str(e)) from e

(502 fits: a downstream resource the daemon depends on — the OAuth connection — is unavailable. Alternatively 422 or 503 with the error message are defensible too, but any explicit status beats the opaque 500.)

The forward_all route is unaffected — it uses a per-provider except Exception loop inside ConnectionForwarder.forward_all that collects and returns errors rather than raising.

…-out routes

The OAuth forward-out feature (#2154) added the /v1/connections intake surface
and bumped the frozen contract to schema 2.5, but the capability-matrix
bookkeeping, the schema-version guard, and the OpenAPI conformance coverage
still tracked 2.4 with 18 REST ops — so every artifact-drift guard failed.

Register the three intake routes (POST/GET/DELETE /v1/connections) as
contract-tested-only ops, update the counts (21 REST functional / 24 in
contract), lock the guard at 2.5, regenerate CAPABILITY_MATRIX.md and
specification.html, and add the routes to the conformance-coverage set
(behavioral conformance already lives in test_forwarded_credentials.py).

@kovtcharov kovtcharov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Rebased onto current main, CI green, no conflicts. Approving.

@kovtcharov
kovtcharov added this pull request to the merge queue Jul 18, 2026
Merged via the queue into main with commit a3db04e Jul 18, 2026
39 checks passed
@kovtcharov
kovtcharov deleted the feat/oauth-forward-out-2154 branch July 18, 2026 06:32
kovtcharov added a commit that referenced this pull request Jul 18, 2026
…#2238/#2239, OAuth forward-out #2203) into refactor/hub-agent-first-layout-2019
@itomek itomek mentioned this pull request Jul 22, 2026
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent::email Email agent changes tests Test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(connectors): OAuth forward-out to sidecars (V2-14)

2 participants