fix(security): pin resolved IP in WebClient to eliminate DNS rebinding #964
Open
theonlychant wants to merge 15 commits intoamd:feature/chat-agent-file-navigationfrom
Open
fix(security): pin resolved IP in WebClient to eliminate DNS rebinding #964theonlychant wants to merge 15 commits intoamd:feature/chat-agent-file-navigationfrom
theonlychant wants to merge 15 commits intoamd:feature/chat-agent-file-navigationfrom
Conversation
## Changes
This pull request introduces a new optional governance layer for GAIA
agents, providing action-level governance (ACGS-lite semantics) with
extension points for future workflow-level features. The governance
system is opt-in and does not affect existing agents unless explicitly
enabled. The changes include the addition of a new `gaia.governance`
package, a comprehensive example agent demonstrating governance
features, and detailed documentation to guide users. The governance
framework is modular, allowing developers to mix in governance
capabilities, tag tools with risk levels, and configure policy engines,
reviewers, and audit logging.
The most important changes are:
**New Governance Framework:**
* Added the `gaia.governance` package, introducing a modular governance
layer for GAIA agents. This includes the `GovernedAgentMixin`,
`GaiaGovernanceAdapter`, risk tagging decorators, and extension points
for policy engines, receipt services, and checkpoint runtimes.
* Implemented the `GaiaGovernanceAdapter` class, which composes policy
evaluation, checkpointing, receipt issuance, and policy version binding
into a single entry point. It ensures secure, auditable, and extensible
governance flows for agent tool calls.
* Provided an `action_mapper` utility to map GAIA tool calls into
governance action requests, standardizing how actions are represented
for policy evaluation.
**Documentation and Examples:**
* Added a comprehensive `README.md` for the `gaia.governance` package,
including quick start instructions, configuration options, security
properties, and extension points. This documentation enables developers
to quickly understand and adopt the governance system.
* Introduced a new example, `examples/governed_weather_agent.py`,
demonstrating how to wrap an agent with governance, define risk-tagged
tools, and handle governance decisions (ALLOW, BLOCK, REVIEW) with local
and MCP tools.
**Packaging:**
* Updated `setup.py` to include the new `gaia.governance` package in the
distribution, ensuring it is installed and available for import.
---
## Hardening & Polish (added in 4 follow-up commits)
Triggered by a PR-review pass that surfaced merge blockers and
architectural feedback. All concerns addressed without expanding feature
scope.
**Merge blockers fixed** — `f242e28 fix(governance): harden error
handling and align docs with additive tags`
* Tightened five `except Exception` sites that were silently swallowing
errors. The most important one (`_resolve_canonical_tool_name`) now logs
unexpected resolver errors with `exc_info=True` instead of falling
through silently. This closes the alias-bypass risk where governance
could check tags on the wrong key when the resolver had a bug. The other
four sites (`_lookup_tool_fn`, `_invoke_callback`, `_prompt_review`,
`JsonlReceiptService._read_all`) now use specific exception types and
log at WARNING.
* `_prompt_review` now returns `(approved, exception_or_None)` so
`_handle_review_checkpoint` can stamp the exception type and message
into the receipt's `metadata.evidence.resolution.reason` (`15bc40b`).
The audit log can now distinguish "reviewer chose no" from "reviewer
crashed" — previously both produced the same boilerplate `"reviewer
rejected"` reason.
* Documentation now matches the code: tag merge is **additive (union,
deduplicated)** — *not* "explicit dict wins". Updated README, the
`@govern` decorator's docstring, and the inline comment in
`mixin._build_action_request` to describe what the tests have always
asserted.
* `_canonical_hash` for BLOCK-receipt evidence now handles non-JSON tool
args, complex types, and cycles without falling back to `repr()`,
keeping receipts deterministically hashable across all inputs.
* `JsonlReceiptService.issue_receipt` now performs strict canonical JSON
validation at issue time, rejecting non-canonical metadata (NaN/Inf,
opaque objects) so tampered or unparseable receipts cannot land in the
audit log.
* Public docs registered: new `docs/sdk/sdks/governance.mdx` plus an
entry in `docs/docs.json` SDK navigation. Closes the missing-docs
blocker.
**CI guard** — `2ed500d ci(test_api): cap job runtime at 30 minutes`
* The API Tests job had no `timeout-minutes` and was hanging for 4+
hours on the in-flight CI run for this PR. Added a 30-minute cap (covers
worst-case Lemonade boot + model pull + tests) so future runs fail fast
on hangs.
**Polish** — `ca941a9 refactor(governance): polish pass — drop dead
code, tighten lock, deep-copy tags`
Driven by a parallel three-agent review (code-reviewer +
architecture-reviewer + test-engineer):
* Deleted `workflow_mapper.py` and
`StaticPolicyBindingService.bind_receipt`. Both were "forward-compat
seams" with zero callers in src/, tests/, examples/, or docs/. They'll
come back in the PR that adds the real event surface, when the actual
signature is known. YAGNI.
* Tightened `JsonlReceiptService.get_receipt`: cache reads/writes were
unsynchronized while a concurrent `issue_receipt` was mutating the same
dict under `_lock`. Both paths are now under the lock.
* `GovernedAgentMixin.__init__` now deep-copies inner risk-tag lists so
a caller cannot mutate the agent's tag table after construction by
holding onto the original list reference.
* Added a comment on the `bool`-before-`int` ordering in
`_canonical_json_value` (subclass relationship — without the order,
`True` would canonicalize as `1`).
* Debug breadcrumb on receipt-log malformed-line skips, so an operator
chasing a missing receipt has something to grep.
**Test additions** — `5cdfee5 test(governance): cover hardened error
paths and fail-closed branches`
Added 6 new tests covering branches that had no regression guard:
* `test_resolver_unexpected_exception_logs_and_governs_raw_name` —
proves a buggy `_resolve_tool_name` raising RuntimeError still triggers
governance on the raw name AND emits an operator-visible warning. Future
regression where the warning is swapped for a silent fallback fails this
test.
* `test_resolver_lookup_error_is_silent_and_governs_raw_name` — proves
the expected "tool not in registry" case (`LookupError`) is absorbed
silently with no log noise.
* `test_unknown_transition_outcome_fails_closed` — proves a custom
`CheckpointRuntime` returning a status the mixin doesn't know is denied,
not let through.
* `test_handle_transition_rejects_unknown_decision_type` — same idea at
the adapter layer for an unknown `GovernanceDecision.decision`.
* `test_read_all_skips_malformed_lines` — proves a corrupt line in the
middle of an audit log doesn't block readers from finding subsequent
valid records.
* Existing callback-exception and reviewer-exception tests gained
`caplog` assertions so a future silent-swallow regression is caught.
Plus two readability fixes: renamed
`test_explicit_dict_overrides_decorated_tags` →
`test_explicit_empty_dict_does_not_downgrade_decorator_tags` (the body
asserted additive semantics, the old name said the opposite); replaced
hardcoded `"test_governance_adapter.SlotOnlyEvidence"` qualname strings
with `f"{Cls.__module__}.{Cls.__qualname__}"` so the tests survive a
file rename.
**Verification (fresh evidence at HEAD `15bc40b`)**
* Governance test suite: **67 passed** (was 27 before the polish — added
5 from the in-flight strict-evidence work and 6 from the polish review).
* `python util/lint.py --black --isort`: PASS.
* No dead code residue: `git grep` of `workflow_mapper`,
`map_gaia_event_to_transition`, `bind_receipt` returns zero matches.
* Public-import smoke test: `GaiaGovernanceAdapter.default()` constructs
with the four expected components.
* Broader unit tests (excl. `tests/unit/chat/` which needs the optional
`[ui]` extra): **946 passed, 16 skipped** — no regressions introduced.
* Upstream merge of `amd:main` (10+ commits including the
YAML-manifest-removal refactor `amd#914`) is incorporated. `_TOOL_REGISTRY`
survived that refactor; governance imports remain green.
**Items intentionally not in this PR** (deferred for follow-up):
* `Agent.__init__` accepting `**kwargs` so multi-mixin composition
(`MCPAgent + GovernedAgentMixin + ApiAgent`) doesn't trip on closed
signatures — touches `agents/base/agent.py` and is a separate concern.
* Public accessor for `_TOOL_REGISTRY` to replace the
`gaia.agents.base.tools._TOOL_REGISTRY` private import in
`mixin._lookup_tool_fn`.
* Extracting `_canonical_hash` and `_canonical_json_value` to a public
`gaia.governance.canonical` module so any conforming
`ReceiptServiceProtocol` can verify or recompute hashes independently.
* `default()` accepting component overrides for `policy_engine`,
`receipt_service`, `checkpoint_runtime`, `policy_binding` so third
parties can swap engines without forgoing the factory.
These are good ideas that expand public API surface and belong in a
focused follow-up PR rather than bundled into this merge.
---
## Governance REVIEW + existing confirmation path
Follow-up for PR review 4197475871: this PR takes Path A. Governance
remains an opt-in policy layer, but REVIEW decisions now reuse GAIA
Agent UI confirmation when the active console advertises
`blocking_confirmation = True` (`SSEOutputHandler`). An explicit
`governance_reviewer` still takes precedence for non-UI or custom
approval flows, and default `AgentConsole` remains fail-closed because
its confirmation method auto-approves.
Regression coverage added:
* Blocking-console fallback: governance REVIEW delegates to
`console.confirm_tool_execution` only for consoles marked
`blocking_confirmation = True`.
* Agent UI path: a governance-tagged REVIEW tool with `SSEOutputHandler`
emits the existing `permission_request` event and runs only after
approval.
* Default-console safety: unmarked consoles are not treated as implicit
reviewers, preserving fail-closed behavior.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: dislovelhl <dislovelhl@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…md#872) ## Summary Mobile Access used to surface raw ngrok stderr (``ERR_NGROK_107``, ``dial tcp ... no such host``, or worst case nothing) when something went wrong, leaving the user no path forward without consulting the docs. This PR parses every common ngrok failure into actionable guidance the modal renders verbatim, plus adds an HttpOnly-cookie auth path so opening the QR-code URL in a mobile browser Just Works. ## Threads - **Friendly tunnel diagnostics** (``tunnel.py``) — preflight ``_check_ngrok_authtoken_configured`` (now honouring ``$NGROK_AUTHTOKEN`` first, then v2 flat / v3 nested config layouts) catches the unconfigured case before spawn; ``_parse_ngrok_error`` matches err codes + English fragments and returns ready-to-paste install/config commands. **Why this matters:** the previous "raw stderr" path made every ngrok failure a docs-search; users now see exactly what to run. - **Cookie-based mobile auth** (``server.py`` + SPA handler) — ``?token=<uuid>`` in the QR URL is converted to an HttpOnly ``gaia_tunnel_token`` cookie on the SPA landing response, so React's same-origin ``fetch('/api/...')`` is authenticated automatically. Bearer header continues to work for headerful clients. **Why this matters:** without this, the mobile browser can't carry the token to subsequent requests without query-string smuggling. - **2 correctness fixes baked in:** ``pkill -f ngrok`` → ``pkill -x ngrok`` (the broad form matched ``vim ngrok.md`` etc.); operator-precedence parens added to the network + TLS branches of ``_parse_ngrok_error`` so ``x509 OR (certificate AND verify)`` is now self-documenting rather than implied. **Why this matters:** the ``-f`` form would kill unrelated user processes; the precedence ambiguity made the parser fragile to reorder. - **UX nit:** sidebar mobile button always opens the modal — stopping is an explicit button inside, so accidental sidebar clicks don't tear down the tunnel mid-scan. ## Test plan - [x] ``pytest tests/unit/chat/ui/test_tunnel.py tests/unit/chat/ui/test_tunnel_auth.py`` (55/55 passing — covers preflight env-var/v2/v3 layouts, parse_ngrok_error positive + negative branches, error-preservation across stop, cookie/header/both/neither auth matrix) - [x] ``cd src/gaia/apps/webui && npm run build`` (clean) - [x] ``python util/lint.py --black --isort`` (clean) - [ ] Manual: ``gaia chat --ui`` → click mobile button → verify each failure path renders friendly text (ngrok not installed, missing authtoken, session limit by spawning a second tunnel) - [ ] Manual: scan QR on phone → React app loads on cookie auth, no token in address bar
## Summary Removes all references to RAUX (the retired Open-WebUI fork) from the documentation. RAUX is no longer part of GAIA; the current UI is the Gaia Agent UI (`src/gaia/apps/webui/` + `src/gaia/ui/`). ## Why `docs/deployment/ui.mdx` still contained a full "GAIA UI (RAUX) Interface" section, including an acknowledgment block for the OpenWebUI team and a link to the retired `aigdat/raux` repository. Three other doc files used the bare "GAIA UI" label (which pointed contributors toward the wrong product). These references misdirect external contributors and are factually wrong. Refs amd#929 ## Changes - `docs/deployment/ui.mdx` — removed the "GAIA UI (RAUX) Interface" section entirely; renamed "GAIA Chat (Lightweight Desktop)" → "Gaia Agent UI Architecture" and removed the "lighter alternative to RAUX" framing; updated Developer Quick Start link text. - `docs/guides/custom-agent.mdx` — "GAIA UI" → "Gaia Agent UI" (line 13). - `docs/plans/axis-gaia-integration.md` — "GAIA UI" → "Gaia Agent UI" (line 162). - `docs/plans/desktop-installer.mdx` — "GAIA UI" → "Gaia Agent UI" (line 419). **Must land before PR 3** (the stale-strings CI workflow), which enforces the absence of these terms going forward. ## Test plan - [ ] `grep -rn -i "raux\|open-webui\|openwebui" docs/` — zero hits - [ ] `grep -rn "GAIA UI" docs/` (without "Agent") — zero hits - [ ] All internal links in `docs/deployment/ui.mdx` resolve: `/guides/agent-ui`, `/sdk/sdks/agent-ui`, `/spec/agent-ui-server`, `/reference/troubleshooting#appimage-on-linux` ## Checklist - [x] I have linked a GitHub issue above (`Refs amd#929`). - [x] I have described **why** this change is being made, not just what changed. - [x] No code changes — docs only; lint and unit tests are not applicable. - [x] Documentation updated (this PR is the documentation update).
…amd#930) ## Summary Refresh the GitHub issue templates, PR template, and `CONTRIBUTING.md` so external contributors arrive with the structure maintainers already enforce in review. First of three PRs for the contributor-onboarding refresh tracked in amd#929. ## Why The current templates are stale and under-enforced. Both issue templates ask whether the bug relates to "GAIA UI (Open-WebUI)" — a UI that no longer exists; the PR template is a four-line `## Changes` stub with no required fields, no linked-issue prompt, and no statement of *why*; and `CONTRIBUTING.md` never states the rule that every PR must reference an issue. The result is the same review-time coaching ("please file an issue first", "please describe why") happening on every external PR. This PR moves those rules into the templates and the contributing guide so reviewers can point at the docs instead of restating them. The PR-template structure mirrors the rules already in [`CLAUDE.md`'s "PR Descriptions — Tight and Value-Focused"](https://github.com/amd/gaia/blob/main/CLAUDE.md#important-pr-descriptions--tight-and-value-focused) section so the rule and the form stay in sync. ## Linked issue Refs amd#929 — first of three PRs. PR 2 (docs cleanup) and PR 3 (regression workflow) follow. ## Changes - **`.github/ISSUE_TEMPLATE/bug_report.yaml`** — streamlined from 13 fields to 6. Replaced "GAIA UI (Open-WebUI)" with "Gaia Agent UI". Consolidated 7 hardware dropdowns/inputs into one freeform Environment textarea (uses `placeholder:` so untouched submissions stay clean). Added optional Acceptance Criteria. `What happened?` is now required. Added "redact tokens/credentials before pasting logs" guidance. - **`.github/ISSUE_TEMPLATE/feature_request.yaml`** — streamlined. Replaced "GAIA UI (Open-WebUI)". Renamed primary field to "What problem are you trying to solve?" (required) so contributors lead with the problem, not a solution. Added optional Proposed solution and Acceptance Criteria fields. - **`.github/pull_request_template.md`** — replaced 4-line stub with structured template: Summary / Why / Linked issue (`Closes #N`) / Changes / Test plan / Checklist. Linked-issue placeholder is visibly `Closes #N` so an unfilled template renders as obvious placeholder rather than a silently-empty `Closes #`. - **`CONTRIBUTING.md`** — revamped as the canonical general guide. Adds a prominent "Before you open a pull request — open an issue first" section that explicitly states the rule, with the rare exceptions (typo fixes, doc-only changes under ~10 lines) called out. - **`docs/reference/contributing-docs.mdx`** — added a Mintlify `<Note>` at the top pointing readers to `CONTRIBUTING.md` for code/issue contributions; the docs-taxonomy guide is otherwise unchanged. ## Test plan - [x] YAML syntax valid: `python3 -c "import yaml; yaml.safe_load(open(...))"` passes for both issue templates. - [x] Stale-string sweep on this PR's surface: `grep -rEn -i "open[-]?webui|GAIA UI" .github/ CONTRIBUTING.md` returns zero matches. (The remaining `docs/` hits are PR 2's scope, tracked in amd#929.) - [x] `code-reviewer` agent reviewed the diff; two suggestions applied (dead `SECURITY.md` link removed; raw-YAML template links swapped for rendered `?template=` URLs). - [x] Adversarial multi-agent reflection (`/reflect-plan`) completed; three auto-amendments applied: bug-report Environment field switched from `value:` → `placeholder:` (HTML-comment template was being submitted into issue bodies), `Closes #` → `Closes #N` for visible-placeholder UX, removed duplicate lint/test code block in `CONTRIBUTING.md` (already linked to `dev.mdx`). - [ ] **Manual render check after merge**: open `https://github.com/amd/gaia/issues/new?template=bug_report.yaml` and `?template=feature_request.yaml` — confirm "Gaia Agent UI" text, no "Open-WebUI", required fields render with red asterisks. - [ ] **Manual PR template check after merge**: next PR opened against `main` should pre-fill with the new structure. ## Checklist - [x] I have linked a GitHub issue above (`Refs amd#929` — `Closes` will be on PR 3 of the series). - [x] I have described **why** this change is being made, not just what changed. - [x] I have run linting and tests locally — N/A for templates/markdown; YAML validity verified directly. - [x] I have updated documentation if user-visible behavior changed (`CONTRIBUTING.md` and `docs/reference/contributing-docs.mdx` updated; `docs/docs.json` nav unchanged because the page IDs are stable).
Fixes amd#934 — GAIA fails to start after a clean install of 0.17.4. ## What was the bug Three layered failures, each masking the next: **1. `ERR_STREAM_WRITE_AFTER_END` → bare Electron crash dialog.** The log-tee stream (`electron-main.log`) was created early in startup. On app exit, `process.on('exit')` called `stream.end()`. If anything then called `console.log()`, the stream emitted an async `'error'` event. Because no `'error'` listener was attached, Node promoted it to `uncaughtException`, which Electron surfaced as a raw JS error dialog with no GAIA branding. **2. `ERR_FAILED (-2)` loading `index.html`.** Even after adding an `'error'` listener to absorb the stream error, the app still crashed with a navigation failure. The URL format was wrong: Node's `url.format()` (used internally by Electron's `loadFile()`) produces `file:///C:\path` with backslashes on Windows. Chromium 130+ (Electron 40) rejects backslash file URLs → `ERR_FAILED (-2)`. **3. `ERR_FAILED (-2)` on a clean install even with correct URL.** After switching to `pathToFileURL()` (which produces forward-slash URLs), the crash still happened on a *truly* fresh install where `~/.gaia` didn't exist. The root cause: after the backend installer's progress dialog is destroyed on install completion, Electron fires `window-all-closed`. At that point `trayManager` hadn't been created yet, so the handler called `app.quit()`. The async cleanup finished instantly (nothing to tear down), fired a second `app.quit()` that `will-quit` didn't prevent, and Electron began tearing down Chromium — right as the startup sequence called `createWindow()` then `loadURL()`. The renderer process was invalidated mid-navigation → `ERR_FAILED (-2)`. ## What we changed - **`main-safety-net.cjs`** (new) — extracted `installSafetyNet` and `installLogTee` from `main.cjs`. `installSafetyNet` registers `uncaughtException`/`unhandledRejection` handlers that write a FATAL entry to the log and show a GAIA-branded error dialog. `installLogTee` attaches a `'error'` listener to the log-tee stream, absorbing write-after-end errors before they reach the global handler. - **`main.cjs`** — switched `loadFile()` → `loadURL(pathToFileURL(...))` for correct forward-slash file URLs on Windows. Added `isBootstrapping` flag (true until `createWindow()` runs) that makes `window-all-closed` a no-op during the install phase, preventing the premature quit race. - **`electron-builder.yml`** — added `main-safety-net.cjs` to the ASAR files list. - **`tests/electron/test_main_error_handling.js`** — 12 Jest tests covering `installSafetyNet` and `installLogTee` (re-entry guard, pre/post-ready dialog branch, crash counter, stream error absorption, etc.). ## How we tested Manual fresh-install test on Windows 11 (uninstall → delete `~/.gaia` → reinstall from the built NSIS `.exe`): - Before: "GAIA crashed" dialog appeared immediately after the backend installer completed; `~/.gaia/electron-main.log` showed `FATAL ERR_FAILED (-2) loading file:///C:/...index.html`. - After: app launched normally, backend connected, chat UI loaded. --------- Co-authored-by: Kalin Ovtcharov <kalin@extropolis.ai>
## Summary Release prep for **v0.17.5** — patch over v0.17.4 covering 27 commits. Bumps `__version__`, syncs the webui `package.json`, adds the release notes, and registers the page in `docs.json`. Lemonade pin remains `10.2.0`. The full notes are in [`docs/releases/v0.17.5.mdx`](docs/releases/v0.17.5.mdx) — highlights: Gemma 4 default with native `tool_calls`, Chat Lite for low-memory hardware, semantic code search via CodeAgent, optional governance layer, Agent UI bundled in the PyPI wheel, friendly ngrok tunnel diagnostics, and a VLM C++ SDK. ## Changes - `docs/releases/v0.17.5.mdx` — new release notes (9 What's New, 7 Bug Fixes, 3 Release/CI, 8 Docs) - `docs/docs.json` — added `releases/v0.17.5` to Releases tab; bumped navbar to `v0.17.5 · Lemonade 10.2.0` - `src/gaia/version.py` — `__version__` `0.17.4` → `0.17.5` - `src/gaia/apps/webui/package.json` — synced to `0.17.5` via `installer/version/bump-ui-version.mjs` ## Test plan - [x] `python util/validate_release_notes.py docs/releases/v0.17.5.mdx` — passes - [x] `node installer/version/bump-ui-version.mjs` — webui package version matches `version.py` - [ ] CI green (lint, unit tests, docs build) - [ ] Reviewer reads the release notes once for tone/accuracy - [ ] On merge: pre-tag verification (Phase 3 of `gaia-release` skill) before tag push --------- Co-authored-by: Tomasz Iniewicz <infancy_shred.0d@icloud.com>
…amd#949) ## Summary Fixes GAIA ignoring the new Gemma default model and falling back to Qwen on Windows 11, causing the wrong model to load in the frontend. ## Why After commit 5d37771 made Gemma-4-E4B the default model, Windows users reported that GAIA still attempts to load Qwen instead. This left the new default model effectively unreachable on Windows, making the frontend unusable for anyone who hadn't manually configured a model. ## Linked issue Closes amd#948 ## Changes - Fixed model selection logic to correctly resolve the new Gemma default on Windows instead of falling back to Qwen ## Test plan - [x] `pytest tests/unit/` - passing locally - [x] `python util/lint.py --all` - no failures - [ ] Manual: launch `gaia chat --ui` on Windows and verify Gemma loads instead of Qwen ## Checklist - [x] I have linked a GitHub issue above (`Closes amd#948`). - [x] I have described **why** this change is being made, not just what changed. - [x] I have run linting and tests locally. - [ ] I have updated documentation if user-visible behavior changed. --------- Signed-off-by: theonlychant <sacehenry@gmail.com>
…ctors framework) (amd#926) Closes amd#915 (when promoted from draft and merged). Self-contained `gaia.connections` module — any GAIA caller (SDK, CLI, AgentUI) can drive the OAuth 2.0 PKCE flow for Google. Refresh tokens land in the OS keychain (macOS Keychain, Windows DPAPI, Linux SecretService); per-agent grants live in `~/.gaia/connections/grants.json`; an agent can only `get_access_token` for scopes the user explicitly granted it. This PR ships as **draft** because it is also the **baseline commit for a larger Connectors framework** (parent issue forthcoming) — the scope expanded after a meeting to make GAIA host many connectors with a Claude-style tile UI. The plan is at `~/.claude/plans/floating-discovering-gray.md`. Keeping this PR draft so reviewers can pull the baseline (157 tests green) before the framework refactor renames `gaia.connections` → `gaia.connectors` and unifies the MCP catalog into the same surface. - **`src/gaia/connections/`** — provider-agnostic core: errors, providers (Google), pkce, store (keyring with backend allowlist + tripwire), grants ledger, async token cache (double-checked locking, 60 s expiry buffer, refresh-token rotation), aiohttp loopback flow, events Protocol, public api, CLI. - **`src/gaia/agents/base/agent.py`** — `REQUIRED_CONNECTIONS` ClassVar; `process_query` wraps tool execution in a private `_agent_context` contextvar so every tool body knows its agent identity. - **`src/gaia/agents/registry.py`** — namespaced agent ids (`builtin:*` / `custom:<sha256>:*`); reserved-id check blocks custom agents from claiming a built-in's id. - **`src/gaia/ui/routers/connections.py`** — thin presentation layer: `/api/connections/{catalog,configure,test,authorize,grants,events,_debug}`. SSE event emitter with bounded queue. `/_debug` gated by `GAIA_DEBUG=1`. - **`src/gaia/cli.py`** + **`src/gaia/connections/cli.py`** — `gaia connections {connect,status,disconnect,grants ...}`. - **`src/gaia/apps/webui/src/components/ConnectionsSection.{tsx,css}`** + supporting store/hook/types — Settings → Connections panel with Connect/Disconnect + per-agent grant toggle. SSE updates the UI within ~2 s of OAuth completion. - **`docs/security/connections.mdx`** — threat model. - **`docs/sdk/infrastructure/connections.mdx`** — SDK reference with three equal-weight sections (SDK / CLI / agent author). - **`docs/runbooks/google-oauth-client.md`** — internal client-id rotation procedure. - **`docs/local-test/`** — recipe + custom test agent for end-to-end Google OAuth verification against a personal account (gated until env var is set). - Refresh tokens never leave the OS keychain. Backend allowlist refuses `PlaintextKeyring` / `EncryptedKeyring` so a Linux user without SecretService gets an actionable error rather than silent plaintext storage. - Per-agent grants prevent prompt-injection-driven scope escalation: even a malicious tool body cannot `get_access_token` for a connector the user did not explicitly grant *that* agent. - `client_id_hash` tripwire invalidates stored tokens after an OAuth client rotation; users reconnect cleanly instead of using stale credentials. - Same primitives serve SDK, CLI, and AgentUI — proven by an explicit multi-caller equivalence integration test. - [x] `python -m pytest tests/unit/connections/ tests/unit/test_agent_required_connections.py tests/integration/test_multi_caller_equivalence.py` — 157 passing. - [x] `python -m black --check src/gaia/connections/ src/gaia/ui/routers/connections.py tests/unit/connections/` — clean. - [x] `python -m isort --check-only src/gaia/connections/ ...` — clean. - [x] `cd src/gaia/apps/webui && npx tsc --noEmit` — zero errors. - [ ] Local Google OAuth E2E against a personal account (deferred to after the connectors framework refactor, per `docs/local-test/README.md`). - [ ] Linux CI keyring matrix (in-memory backend autouse fixture covers the unit suite; `gnome-keyring` integration job is a follow-up). - The library is self-contained — every test runs without the AgentUI server and without a real keyring (in-memory backend in `tests/unit/connections/conftest.py`). - `_agent_context` is intentionally **private** (not re-exported in `gaia.connections.__init__`). A tool body cannot import it to forge an agent identity. Custom agents get an origin-hashed namespaced id so a custom agent declaring a built-in's `AGENT_ID` does not inherit prior grants. - Code-reviewer agent ran on the diff during development; 5 findings reported, 4 fixed (asyncio-run-in-running-loop guard on the sync wrapper, consent-denied response now serves the rejection page instead of the success page, `connected_at` populated from `time.time()` not from the absent token-response field, BuilderAgent / CodeAgent overrides updated so they cannot bypass the agent-context binding). The 5th was the v1 single-account-per-provider intentional limit — strengthened the docstring instead of changing behavior.
…work (amd#906) Adds a third issue template (alongside `bug_report.yaml` and `feature_request.yaml`) specifically for team-internal feature work and tasks intended for coding-agent assignment. ## Why The existing templates are user-facing. With AGENTS.md (PR amd#904) establishing "spec-before-PR" as a rule for consumer-critical work, internal issues need a template that prompts authors to capture: Goal, Scope, Acceptance criteria, Attribution / prior art, Dependencies, Failure modes, plus capability domain and product track selection. ## What it captures - **Goal** + **Scope** + **Acceptance criteria** sections (required) matching the depth of amd#887/amd#888/amd#890 specs - **Attribution** section (per CLAUDE.md attribution rule) - **Failure modes** section (per CLAUDE.md no-silent-fallback rule) - **Domain dropdown** — 8 options matching the new `domain:*` label taxonomy - **Track dropdown** — 3 options matching `track:*` labels (consumer-app / oem-pc / platform) - **Priority dropdown** with explicit definitions (p0=4 weeks, p1=2 milestones, etc.) - **Consumer-critical** checkbox ## Cross-references - AGENTS.md (PR amd#904) establishes the rules this template enforces in practice - Mobile design-system spec (PR amd#905) is an example of the spec depth required for consumer-critical work ## Test plan - [ ] Template renders correctly in the GitHub "New issue" picker - [ ] All dropdowns work - [ ] Required fields enforce on submit - [ ] No conflict with existing bug_report or feature_request templates
The existing PR-description guidance in CLAUDE.md was directionally
right ("tight and value-focused") but loose enough that a recent PR
(amd#946 / amd#944) still shipped with a "What changed" enumeration the diff
already showed and a "Summary" section that buried the user-observable
impact behind implementation details. Future agents reading the file
would do the same.
After this change the default shape is just two sections — "Why this
matters" (with required before/after framing) and "Test plan" — with a
"user-observable impact in <30s without reading the diff" litmus check,
and three new anti-patterns lifted directly from the patterns the prior
PR exhibited.
## Test plan
- [x] No code changed; doc-only edit
- [x] Re-read the new section against amd#946 to confirm the prior
description fails the new rules
…g TOCTOU Signed-off-by: theonlychant <sacehenry@gmail.com>
Signed-off-by: theonlychant <sacehenry@gmail.com>
2ee1d7b to
1e3ad62
Compare
Signed-off-by: theonlychant <sacehenry@gmail.com>
Signed-off-by: theonlychant <sacehenry@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a DNS rebinding TOCTOU vulnerability in
WebClient._validate_host_ip()by pinning the resolved IP at validation time and reusing it for the
actual connection, eliminating the window between DNS resolution and connect.
Why
WebClientresolved the hostname twice - once in_validate_host_ip()for SSRF validation, and again when
requestsconnected. A low-TTL DNSrebind could pass validation with a public IP then connect to an RFC1918
address (e.g.
127.0.0.1,169.254.169.254). Identified during reviewof #495.
Linked issue
Closes #956
Refs #495
Changes
_validate_host_ip()now returns the resolved IP string instead of None_PinnedIPAdapter- a customHTTPAdapterthat forces connectionsto the pre-resolved IP while preserving the original
Hostheader_request()mounts_PinnedIPAdapterafter validation so the same IPis used for both the SSRF check and the actual TCP connect
Test plan
pytest tests/unit/- all passingpython util/lint.py --all- no failuresChecklist
Closes #956).