fix(harbor): require an admin key on the capture proxy control plane

[burtenshaw]

What was exposed

The capture proxy's session-management routes are gated by _admin_ok(), which admits every caller when app.state.admin_key is unset:

• POST /sessions — mint a session key the proxy then honours (open relay to the upstream engine)
• GET /sessions — enumerate every live rollout
• GET /sessions/{id} — read session metadata
• GET /sessions/{id}/rollout — read a rollout's full token-level training data
• GET /sessions/{id}/trace_entries — read the trace
• DELETE /sessions/{id} — end someone else's rollout

Why it reached users

run_batch() in src/openenv/harbor/runner.py (behind openenv harbor rollout) built its CaptureServer without an admin_key, then published it through make_forwarder(expose) — and expose defaults to "gradio", i.e. a public tunnel URL. The standalone python -m openenv.core.harness.capture.server had no --admin-key flag at all. HarborService (harbor serve/push) already handled this correctly via $OPENENV_CAPTURE_ADMIN_KEY with a random fallback; rollout and the CLI did not.

What changed

• run_batch() takes admin_key: str | None = None, resolved as explicit argument → $OPENENV_CAPTURE_ADMIN_KEY → secrets.token_urlsafe(32), and passes it to CaptureServer. It prints a note that the session routes are gated (never the key). The rollout path uses the in-process registry and the agent only touches the data plane, so no caller needs the key.
• The capture server CLI gains --admin-key (default $OPENENV_CAPTURE_ADMIN_KEY) forwarded to create_app.
• create_app's admin_key docstring now names rollout alongside serve.

Testing

• New tests in tests/envs/test_harbor_runner_lifecycle.py (key always present and ≥32 chars, fresh per batch, explicit arg beats env var, env var honoured) and tests/envs/test_harbor_capture_server.py (--admin-key reaches the app, env-var default, unset stays None for a private port). All 7 fail on main and pass here.
• pytest tests/envs/test_harbor*.py tests/envs/test_capture*.py: 541 passed, 2 skipped.
• usort format, ruff format, ruff check clean.

This is a 0.5.0 release blocker tracked on #1190.

@adithya-s-k could you review this? It follows the same admin-key handling you added in serving.py for harbor serve/push.

  

[cursor]

Stale comment

Release-manager review at head f327f179 — this is the forward-fix Ben asked for after confirming 0.5.0.

Verdict: LGTM (pending exact-head CI + @adithya-s-k)

Closes the release-gating hole: run_batch() always resolves admin_key as arg → $OPENENV_CAPTURE_ADMIN_KEY → secrets.token_urlsafe(32) and passes it to CaptureServer, matching HarborService. The standalone CLI gains --admin-key (env default). Regression tests pin that CaptureServer is never built without a key on the public-tunnel path, and that keys are fresh per batch.

CLI leaving admin_key unset when neither flag nor env is set is intentional for a private local port — help text warns correctly. Not a release concern for rollout/run_batch.

Local: focused harbor capture/runner tests pass on this tree once deps resolve.

Release plan once this merges: refresh draft #1190 onto main, exact-head CI, TestPyPI, then merge/tag/publish 0.5.0. Will not publish while this is open.

View PR

  

_{Sent by Cursor Automation: Release}

[bot-ci-comment]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

[cursor]

Stale comment

Update: REQUEST_CHANGES — I am withdrawing the earlier LGTM on the CLI half.

run_batch() still looks correct and closes the Harbor/openenv harbor rollout release gate.

The standalone CLI does not: --host still defaults to 0.0.0.0, and this PR adds --admin-key that defaults to unset (plus a test that asserts admin_key is None). A default python -m openenv.core.harness.capture.server … therefore still publishes /sessions* without auth on every interface. The “private local port” rationale in the new test is wrong for this CLI’s defaults.

Please fix before merge (loopback default + refuse non-loopback without a key, or mint a key for non-loopback), with a matching regression. @adithya-s-k — still want your eyes on the run_batch path; the CLI fail-open should not ship as-is.

Head after main refresh: f7880a25. 0.5.0 publish still waits on this PR.

  

_{Sent by Cursor Automation: Release}

[cursor]

Alignment Review Report

PR #1193 — fix(harbor): require an admin key on the capture proxy control plane

Automated Checks

• Lint: PASS — ruff format --check, ruff check, and usort check are all clean on the 4 changed files (capture/server.py, harbor/runner.py, and the two test files).
• Debug code: CLEAN — check-debug.sh lists print() in harbor/runner.py and capture/server.py, but those are intentional CLI report output (the adjacent pre-existing print(f" capture :{port}...") at runner.py:136, and server.py:1429/1431). The one new print (runner.py:137) matches that pattern; no breakpoints/pdb.
• Tests: 15/15 pass — test_harbor_capture_server.py + test_harbor_runner_lifecycle.py.

What the change does

_admin_ok returns True for every caller while app.state.admin_key is unset (capture/server.py:698-700). Two publicly-reachable entry points previously left the key unset:

• run_batch (harbor/runner.py, behind the openenv harbor rollout CLI) publishes the proxy on a public tunnel; it now resolves admin_key = explicit arg → $OPENENV_CAPTURE_ADMIN_KEY → random secrets.token_urlsafe(32) and hands it to CaptureServer.
• The standalone capture-server CLI (python -m openenv.core.harness.capture.server) had no way to set the key at all; it now takes --admin-key (default $OPENENV_CAPTURE_ADMIN_KEY).

HarborService (openenv harbor serve) already did this. Net effect: POST /sessions (mint an upstream relay key), GET /sessions[/{id}/rollout] (token-level training data), and DELETE are no longer open on a public URL. Well-scoped, real security fix.

Open RFCs Context

• RFC 012 — Harbor capture purpose, provider fidelity and live-session ownership (In Review, @adithya-s-k) directly governs this area (it documents Harbor PR #1036). Its model keeps "capture purpose, upstream credentials, sampling policy and live traces … scoped to their own session" and treats the shared session registry as a privileged surface. This PR reinforces that model — the admin-key gate is the enforcement of the control-plane/data-plane split 012 describes.
• RFC 005 — Agentic Harnesses (In Review) is the parent RFC for the capture path; unaffected here.

Tier 1: Fixes Required

None. Lint clean, no debug code, tests pass, imports present (os/secrets in both files), and no credential exposure — the auto-minted key is deliberately never logged (the status line only names the env var to set), upholding the No credential exposure invariant.

Tier 2: Alignment Discussion

Principle / Invariant alignment

Positive. Aligns with No credential exposure and the Dual API boundary / agent-isolation invariants (trainer control plane vs. agent data plane). No conflicts.

RFC Conflicts

None. RFC 012 covers this surface and the change hardens rather than violates it.

ALIGNMENT FLAG (non-blocking, RFC context): admin-key control-plane gate in an In-Review RFC area

• RFC at stake: RFC 012 (Harbor capture / live-session ownership), In Review
• The concern: not a conflict — the PR reinforces 012's session-scoping model, but 012 doesn't yet mention the admin-key mechanism itself. Worth folding the control-plane gate into 012 so the trust boundary is documented, not only implemented.
• Suggested reviewers: @adithya-s-k (RFC 012 author); @Darktex (security invariants — INVARIANTS.md "No credential exposure" / Dual API boundary)

Non-blocking observation

The standalone server CLI still defaults --host to 0.0.0.0 (line 1369) while --admin-key defaults unset, so a directly-run standalone proxy on a network-reachable host is open until the operator sets the key. The PR strictly improves the prior state (no key option existed before) and the help text warns about exactly this, so it's acceptable as-is — but defaulting --host to 127.0.0.1, or refusing/auto-minting when binding a non-loopback host without a key, would make the safe path the default. (serve/rollout already auto-mint.)

Summary

• 0 mechanical issues to fix (Tier 1 clean)
• 1 alignment point for humans — non-blocking RFC-012 context (document the admin-key gate) + 1 optional CLI-host hardening idea
• 0 RFC conflicts (RFC 012 reinforced, not violated)

  

_{Sent by Cursor Automation: Pre-review}

[cursor]

Correct fix, and the resolution order reads right: explicit arg wins, then $OPENENV_CAPTURE_ADMIN_KEY (the same var HarborService/serve read, so one setting covers every entry point), then a random per-batch key. Since _admin_ok admits everyone when the key is empty (capture/server.py:698-700) and the forwarder puts this on a public URL, the old unset default was an open control plane — this closes it. Good that the minted key is never printed (upholds No credential exposure); rollouts don't need it because they go through the in-process registry, not /sessions*.

Micro-nit (ignore): HarborService mints token_urlsafe(24) vs 32 here — both are plenty strong, just flagging the byte-count difference next to the "Same resolution as HarborService" comment.

[cursor]

Good that the standalone CLI can finally gate these routes — before this, a port published from python -m ...capture.server had no way to protect /sessions*. One residual footgun: --host (line 1369) defaults to 0.0.0.0 while --admin-key defaults unset, so a directly-run server on a reachable host stays open until the operator sets the key. Non-blocking and the help text warns — but defaulting --host to 127.0.0.1, or refusing/auto-minting on a non-loopback bind without a key, would make the safe path the default here too.

[cursor]

Stale comment

Release-manager re-review at exact head f7880a25.

The run_batch() fix is sound: it always resolves a non-empty admin key before publishing the capture proxy, and its regressions cover explicit/env/generated keys. All exact-head CI checks are green.

One release blocker remains in the standalone capture CLI. The prior assessment called its no-key default a “private local port,” but --host actually defaults to 0.0.0.0. With neither --admin-key nor $OPENENV_CAPTURE_ADMIN_KEY, the new code passes admin_key=None; _admin_ok() then admits every caller to /sessions* on every reachable interface. The new test explicitly pins that fail-open combination.

Please either mint/require a key by default, or default to loopback and refuse a no-key non-loopback bind, with a regression for the default invocation. Keep #1190 held until this is fixed and @adithya-s-k approves the security behavior.

  

_{Sent by Cursor Automation: Release}

[cursor]

Release-manager re-review at exact head fa0af879 — all 11 CI checks green.

Verdict: LGTM

CLI fail-open is closed: when --admin-key / $OPENENV_CAPTURE_ADMIN_KEY is unset, non-loopback binds (including the default --host 0.0.0.0) mint secrets.token_urlsafe(32) and print it before create_app. Loopback-only binds may still leave the key unset. Flag and env still win.

run_batch() path unchanged and still always resolves a non-empty admin key before publishing the capture proxy.

Regressions cover mint-on-default, mint-on-explicit-0.0.0.0, and loopback-unset. Ready to squash-merge once @adithya-s-k (or Ben) approves.

View PR

  

_{Sent by Cursor Automation: Release}