feat(security): federation token + /api operator ceiling; operator goal channel (ADR 0066)

[mabry1985]

🔒 Security-critical — held as DRAFT for your review before merge. Touches the auth middleware.

What & why

Goal trust-gate Phase 2, Option B (the one you picked). Phase 1 (#1492) closed the RCE-via-chat hole by refusing command/test/ci/data+expr goal verifiers from a /goal chat message for everyone — including the operator, who lost a legitimate feature ("keep going until pytest -q passes"). Phase 2 gives that power back through a dedicated operator channel, and adds the path ceiling that makes a federation token real. Full design: docs/adr/0066-goal-trust-operator-channel.md.

The mechanism

• auth.federation_token (optional; env A2A_FEDERATION_TOKEN). The middleware classifies each request by which secret matched (constant-time hmac.compare_digest) → operator | federation.
• R1 path ceiling: a federation credential is denied the whole /api operator surface (plugin install/enable = host code-exec, config/SOUL rewrite, subagent runs, the operator goal set-path) with 403. /a2a + /v1 stay open to either tier. The ceiling lives entirely in the middleware — a request that reaches an /api handler is operator-tier by construction, so no per-request trust has to be threaded into handlers or the streaming producer (this is Option B's simplification over A's fragile propagation).
• Operator goal channel: GoalController.set_goal_operator accepts any verifier type; POST /api/goals routes to it (was plugin-only). Safe because /api is operator-tier by the ceiling. set_goal_safe (agent/SDK/plugin) stays plugin-only.

Backward compatibility (opt-in, fail-safe)

No federation_token configured ⇒ single-token mode, byte-for-byte unchanged: the bearer check is exactly the old one and the ceiling never fires.

The one judgement call to sanity-check (ADR D4): loosening POST /api/goals to accept command verifiers is safe because in single-token mode any bearer holder already has host code-exec via /api/plugins/install — so the goal endpoint adds no new capability; the ceiling (not the goal handler) is the real control. If you'd rather keep defense-in-depth and only accept dangerous verifiers when a federation token is actually configured, that's a ~2-line guard I can add — flag it in review.

Tests

test_a2a_auth.py: federation denied /api (+ /active/<slug>/api, /agents/<slug>/api proxied variants) / allowed /a2a+/v1; operator full access; invalid token still 401 (not 403); single-token + open-mode unchanged; _requires_operator classification. test_goal_controller.py: set_goal_operator accepts dangerous verifiers, rejects unknown, requires condition. Config-roundtrip golden + REST-handler contract updated. 379 passed, ruff clean.

Follow-up slices (ADR 0066)

• PR2 — CLI goal set (thin client of POST /api/goals).
• PR3 — console Goals set-form (DRAFT, local-test gate) — the set-path UI deferred from the goal.iteration work.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 21 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 46375cc3-197d-479a-8e42-4746d39abdef

📥 Commits

Reviewing files that changed from the base of the PR and between 28b3e87 and b5cd03d.

📒 Files selected for processing (12)

• a2a_impl/auth.py
• docs/adr/0066-goal-trust-operator-channel.md
• docs/reference/configuration.md
• graph/config.py
• graph/goals/controller.py
• operator_api/console_handlers.py
• plugins/docs/nav.json
• server/__init__.py
• tests/test_a2a_auth.py
• tests/test_config_roundtrip.py
• tests/test_goal_controller.py
• tests/test_goal_set_programmatic.py

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/goal-trust-phase2-federation-token

---

_{Comment @coderabbitai help to get the list of available commands.}

[protoquinn]

QA Audit — PR #1503 | feat(security): federation token + /api operator ceiling; operator goal channel (ADR 0066)

VERDICT: WARN (non-terminal CI — re-review on green)

---

CI Status

• 8 checks queued: Lint, build, gitleaks, Python tests, Verify workspace config, A2A live smoke, Web E2E, Fleet integration
• ⏳ All non-terminal — PASS/FAIL deferred

Diff Review

• a2a_impl/auth.py: Adds _FEDERATION token, _requires_operator() ceiling, constant-time tier classification, trust_tier on request state. Design is sound: ceiling in middleware, no per-handler trust threading needed.
• docs/adr/0066-goal-trust-operator-channel.md: Clean ADR — Option B rationale, R1/R5 design decisions, backward-compat guarantees.
• Remaining ~395 lines (truncated) include goal controller + tests.

Observations

• Gap: clawpatch structural review unavailable (502 — checkout cache). Proceeding with diff-based review. Recommend a human follow-up structural pass on auth paths.
• LOW: _requires_operator uses substring /api/ matching (line ~158). Covers fleet-proxy variants correctly per ADR, but would false-positive on any future /v1 route containing /api/ as a substring. Consider an allowlist of known prefixes for defense-in-depth — not a blocker today.
• LOW: D4 judgement call — consider the ~2-line guard (only accept dangerous verifiers when federation_token is actually configured) as defense-in-depth. The current logic is correct (no new capability in single-token mode) but the guard is cheap insurance.
• Design quality is high: constant-time classification doesn't leak tier via timing, misconfiguration warnings are appropriate, ceiling placement is correct (before call_next).

— Quinn, QA Engineer

[protoquinn]

Submitted COMMENT review on protoLabsAI/protoAgent#1503.

[protoquinn]

CI terminal-green, no blockers on prior review — auto-approving on green (#748).