didericis/bot-bottle
1
0
Fork 0
Code Issues 32 Pull Requests 8 Actions Packages Releases Wiki Activity

refactor(orchestrator): uniform control-plane auth provisioning per trust domain #482

Merged
didericis merged 3 commits from didericis/refactor/control-plane-auth-provisioning-476 AGit into main 2026-07-25 21:48:47 -04:00
Conversation 7 Commits 3 Files Changed 16
+440 -48
didericis commented 2026-07-25 20:43:07 -04:00
Owner
Copy Link
Copy Source

Closes #476.

Hoists control-plane auth provisioning out of the per-backend launchers into one shared contract, parameterized per trust domain (bot_bottle/trust_domain.py):

  • TrustDomain — one credential boundary (host-canonical key file + role set + env vars). mint/verify are scoped to the domain's roles, so a future host-controller domain (#468) uses its own key/verifier/roles rather than a host role on the control plane's frozenset.
  • ControlPlaneProvisioning — the single seam answering the four invariants: host-canonical key, split key-vs-token credential, CLI token valid across co-running backends, and fail-closed (no open mode) for any co-located topology.
  • Topology — the backend declares what it is; default is co-located + fail-closed, so a backend need not redeclare it.

The Orchestrator ABC gets control_plane_key() (fail-closed) and routes mint_gateway_token() through the contract; the docker/macOS/firecracker orchestrators, the server (verify), and the host CLI client (mint cli) all go through the domain instead of reading the host key directly. orchestrator_auth gains an optional roles= arg (default unchanged) and paths.host_signing_key(filename) generalizes host_orchestrator_token.

Durable rationale in docs/prds/prd-new-control-plane-auth-provisioning.md. Based on fix/db-off-data-plane-469 (#471), which #476 depends on.

Closes #476. Hoists control-plane auth provisioning out of the per-backend launchers into one shared contract, **parameterized per trust domain** (`bot_bottle/trust_domain.py`): - `TrustDomain` — one credential boundary (host-canonical key file + role set + env vars). `mint`/`verify` are scoped to the domain's roles, so a future host-controller domain (#468) uses its own key/verifier/roles rather than a `host` role on the control plane's frozenset. - `ControlPlaneProvisioning` — the single seam answering the four invariants: host-canonical key, split key-vs-token credential, CLI token valid across co-running backends, and fail-closed (no open mode) for any co-located topology. - `Topology` — the backend declares what it is; default is co-located + fail-closed, so a backend need not redeclare it. The `Orchestrator` ABC gets `control_plane_key()` (fail-closed) and routes `mint_gateway_token()` through the contract; the docker/macOS/firecracker orchestrators, the server (verify), and the host CLI client (mint `cli`) all go through the domain instead of reading the host key directly. `orchestrator_auth` gains an optional `roles=` arg (default unchanged) and `paths.host_signing_key(filename)` generalizes `host_orchestrator_token`. Durable rationale in `docs/prds/prd-new-control-plane-auth-provisioning.md`. Based on `fix/db-off-data-plane-469` (#471), which #476 depends on.
didericis commented 2026-07-25 21:08:30 -04:00
Author
Owner
Copy Link
Copy Source

@didericis-claude the wording in the PRD and in the comments is very generic and verbose. Can you tighten it up and talk about the specific SERVICES (orchestrator, host controller) instead of using generic terms, and what specifically this buys us?

The main reason I approved this is I didn't want the orchestrator minting the keys it uses to communicate with the host controller. That's a good change. But the way this is described is confusing.

@didericis-claude the wording in the PRD and in the comments is very generic and verbose. Can you tighten it up and talk about the specific SERVICES (orchestrator, host controller) instead of using generic terms, and what specifically this buys us? The main reason I approved this is I didn't want the orchestrator minting the keys it uses to communicate with the host controller. That's a good change. But the way this is described is confusing.
didericis-claude commented 2026-07-25 21:14:38 -04:00
Collaborator
Copy Link
Copy Source

Good call — reworded the PRD and the docstrings around the concrete services and lead with the specific win.

The framing is now: the orchestrator holds the control-plane key and mints the gateway's/CLI's tokens; the host controller (#468) gets a separate key the orchestrator never holds — so the orchestrator can't mint the credentials it uses to talk to the host controller that starts and stops it. The PRD's Problem section opens with exactly that (adding a host role to the orchestrator's key is the anti-pattern this rules out), and the second win — retiring the hand-rolled per-backend provisioning behind PR #471 — is stated as secondary.

Cut the generic "credential boundary" language throughout trust_domain.py, orchestrator_auth.mint, and paths.host_signing_key in favor of "the orchestrator's key" / "a separate service's key". No behaviour change — docs and comments only (784e492).

Good call — reworded the PRD and the docstrings around the concrete services and lead with the specific win. The framing is now: **the orchestrator holds the control-plane key and mints the gateway's/CLI's tokens; the host controller (#468) gets a separate key the orchestrator never holds — so the orchestrator can't mint the credentials it uses to talk to the host controller that starts and stops it.** The PRD's Problem section opens with exactly that (adding a `host` role to the orchestrator's key is the anti-pattern this rules out), and the second win — retiring the hand-rolled per-backend provisioning behind PR #471 — is stated as secondary. Cut the generic "credential boundary" language throughout `trust_domain.py`, `orchestrator_auth.mint`, and `paths.host_signing_key` in favor of "the orchestrator's key" / "a separate service's key". No behaviour change — docs and comments only (784e492).
didericis-claude changed target branch from fix/db-off-data-plane-469 to main 2026-07-25 21:33:10 -04:00
didericis added 2 commits 2026-07-25 21:33:20 -04:00
refactor(orchestrator): uniform control-plane auth provisioning per trust domain 45f3cefbc5 
Hoist control-plane auth provisioning out of the per-backend launchers into
one shared contract, parameterized per trust domain (#476). Every blocking
finding in PR #471 was the same integration-bug class: each launcher
re-derived, by hand, how to generate the signing key, scope it to the
orchestrator, mint the gateway JWT, and keep the host key canonical.

Introduces `trust_domain.py`:

  * `TrustDomain` — one credential boundary (host-canonical key file + role
    set + env vars). `mint`/`verify` are scoped to the domain's roles, so a
    future host-controller domain (#468) uses its own key/verifier/roles
    rather than a `host` role on the control plane's frozenset (which the
    orchestrator key could then forge).
  * `ControlPlaneProvisioning` — the single seam answering the four
    invariants: host-canonical key, split key-vs-token credential, CLI token
    valid across co-running backends, and fail-closed (no open mode) for any
    co-located topology.
  * `Topology` — the backend declares what it is; the default is co-located +
    fail-closed, so a backend need not redeclare it.

The `Orchestrator` ABC gets `control_plane_key()` (fail-closed) and routes
`mint_gateway_token()` through the contract; docker/macOS/firecracker
orchestrators, the server (verify), and the host CLI client (mint cli) all go
through the domain instead of reading the host key directly. `orchestrator_auth`
gains an optional `roles=` arg (default unchanged) so a domain scopes its own
role set; `paths.host_signing_key(filename)` generalizes host_orchestrator_token.

Adds unit coverage for the domain boundary + provisioning invariants and a PRD
capturing the durable rationale. No change to the auth primitive's HMAC, the
plane split, or the server's documented open-mode fallback.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Closes #476
docs(orchestrator): tighten auth-provisioning wording to concrete services
test / integration-docker (pull_request) Successful in 18s
Details
tracker-policy-pr / check-pr (pull_request) Successful in 21s
Details
test / integration-firecracker (pull_request) Successful in 3m51s
Details
test / unit (pull_request) Failing after 11m53s
Details
test / coverage (pull_request) Has been skipped
Details
test / publish-infra (pull_request) Has been skipped
Details
8f6148d571 
Address review on #482: drop the generic "credential boundary" framing in the
PRD and docstrings and talk about the specific services — the orchestrator holds
the control-plane key; the host controller (#468) gets a separate key the
orchestrator never holds, so the orchestrator can't mint the credentials it uses
to talk to the host controller that owns its lifecycle. Lead with that concrete
win. No code behaviour change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
didericis force-pushed didericis/refactor/control-plane-auth-provisioning-476 from 784e49239d to 8f6148d571 2026-07-25 21:33:20 -04:00 Compare
didericis-codex requested changes 2026-07-25 21:34:27 -04:00
didericis-codex left a comment
Collaborator
Copy Link
Copy Source

The current backends are wired consistently and the focused auth/backend tests pass, but I think the new topology opt-out weakens the invariant it is meant to encode.

Blocking: ControlPlaneProvisioning.orchestrator_key() allows an empty key whenever data_plane_shares_control_host=False, and test_orchestrator_key_allows_empty_when_isolated makes that behavior contractual. Being on a different host does not prevent the gateway from reaching the orchestrator—in fact the gateway must reach /resolve—so an open orchestrator would still treat that gateway as ROLE_CLI and expose CLI-only routes. The safety condition is network isolation/authorization, not host co-location.

Please either remove the empty-key opt-out and always fail closed for provisioned deployments, or model and enforce a stronger condition that proves untrusted/data-plane callers cannot reach the control-plane listener. Merely declaring a separate host should not disable authentication.

Verification: python3 -m unittest -v tests.unit.test_trust_domain tests.unit.test_orchestrator_auth tests.unit.test_orchestrator_client tests.unit.test_docker_orchestrator tests.unit.test_macos_orchestrator tests.unit.test_firecracker_orchestrator (91 tests, all passed).

The current backends are wired consistently and the focused auth/backend tests pass, but I think the new topology opt-out weakens the invariant it is meant to encode. **Blocking:** `ControlPlaneProvisioning.orchestrator_key()` allows an empty key whenever `data_plane_shares_control_host=False`, and `test_orchestrator_key_allows_empty_when_isolated` makes that behavior contractual. Being on a different host does not prevent the gateway from reaching the orchestrator—in fact the gateway must reach `/resolve`—so an open orchestrator would still treat that gateway as `ROLE_CLI` and expose CLI-only routes. The safety condition is network isolation/authorization, not host co-location. Please either remove the empty-key opt-out and always fail closed for provisioned deployments, or model and enforce a stronger condition that proves untrusted/data-plane callers cannot reach the control-plane listener. Merely declaring a separate host should not disable authentication. Verification: `python3 -m unittest -v tests.unit.test_trust_domain tests.unit.test_orchestrator_auth tests.unit.test_orchestrator_client tests.unit.test_docker_orchestrator tests.unit.test_macos_orchestrator tests.unit.test_firecracker_orchestrator` (91 tests, all passed).
didericis commented 2026-07-25 21:37:29 -04:00
Author
Owner
Copy Link
Copy Source

@didericis-claude remove the empty-key opt-out @didericis-codex identified

@didericis-claude remove the empty-key opt-out @didericis-codex identified
didericis added 1 commit 2026-07-25 21:40:25 -04:00
refactor(orchestrator): fail closed unconditionally, drop topology opt-out
tracker-policy-pr / check-pr (pull_request) Successful in 7s
Details
test / integration-docker (pull_request) Successful in 15s
Details
test / unit (pull_request) Successful in 39s
Details
test / integration-firecracker (pull_request) Successful in 3m55s
Details
test / coverage (pull_request) Successful in 22s
Details
test / publish-infra (pull_request) Has been skipped
Details
prd-number / assign-numbers (push) Failing after 21s
Details
test / integration-docker (push) Successful in 22s
Details
lint / lint (push) Successful in 56s
Details
Update Quality Badges / update-badges (push) Successful in 53s
Details
test / unit (push) Successful in 1m52s
Details
test / integration-firecracker (push) Successful in 5m2s
Details
test / coverage (push) Successful in 16s
Details
test / publish-infra (push) Successful in 1m56s
Details
e53104d5c1 
Remove the empty-key opt-out codex flagged: ControlPlaneProvisioning
no longer lets the orchestrator start without a signing key when a
backend declares an "isolated" topology. Host separation is not the
safety condition — the gateway (and any other caller) must reach the
control-plane listener for /resolve, so an open orchestrator would
still grant them full `cli`. The signing key is now mandatory for
every backend.

Drops the now-purposeless Topology/COLOCATED machinery (no backend
declared a non-default topology) and the contractual
test_orchestrator_key_allows_empty_when_isolated. Updates the PRD's
invariant 4 and lifecycle docstring accordingly. Docs/behaviour only
otherwise.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
didericis-claude commented 2026-07-25 21:40:38 -04:00
Collaborator
Copy Link
Copy Source

Done — removed the empty-key opt-out entirely (e53104d).

ControlPlaneProvisioning.orchestrator_key() now fail-closes unconditionally: no signing key → ProvisioningError, for every backend. @didericis-codex was right that host separation is not the safety condition — the gateway must reach the control plane for /resolve, so an open orchestrator would still hand any caller that reaches it full cli. Declaring a separate host no longer disables auth.

Since the Topology / data_plane_shares_control_host machinery existed only to gate that opt-out (no backend ever declared a non-default topology), I dropped it along with the contractual test_orchestrator_key_allows_empty_when_isolated. PRD invariant 4 and the lifecycle docstring now state the key is mandatory with no topology opt-out. Auth/backend suite: 90 tests, all pass.

Done — removed the empty-key opt-out entirely (e53104d). `ControlPlaneProvisioning.orchestrator_key()` now fail-closes unconditionally: no signing key → `ProvisioningError`, for every backend. @didericis-codex was right that host separation is not the safety condition — the gateway must reach the control plane for `/resolve`, so an open orchestrator would still hand any caller that reaches it full `cli`. Declaring a separate host no longer disables auth. Since the `Topology` / `data_plane_shares_control_host` machinery existed only to gate that opt-out (no backend ever declared a non-default topology), I dropped it along with the contractual `test_orchestrator_key_allows_empty_when_isolated`. PRD invariant 4 and the lifecycle docstring now state the key is mandatory with no topology opt-out. Auth/backend suite: 90 tests, all pass.
didericis-codex approved these changes 2026-07-25 21:43:53 -04:00
didericis-codex left a comment
Collaborator
Copy Link
Copy Source

Re-reviewed at e53104d. The previous blocking issue is resolved: the topology-based empty-key opt-out has been removed, and ControlPlaneProvisioning.orchestrator_key() now fails closed unconditionally. The corresponding unsafe test contract was removed and replaced with coverage for the unconditional failure.

I found no further actionable issues.

Verification:

  • git diff --check origin/main...HEAD
  • python3 -m unittest -v tests.unit.test_trust_domain tests.unit.test_orchestrator_auth tests.unit.test_orchestrator_client tests.unit.test_docker_orchestrator tests.unit.test_macos_orchestrator tests.unit.test_firecracker_orchestrator (90 tests, all passed)
Re-reviewed at e53104d. The previous blocking issue is resolved: the topology-based empty-key opt-out has been removed, and `ControlPlaneProvisioning.orchestrator_key()` now fails closed unconditionally. The corresponding unsafe test contract was removed and replaced with coverage for the unconditional failure. I found no further actionable issues. Verification: - `git diff --check origin/main...HEAD` - `python3 -m unittest -v tests.unit.test_trust_domain tests.unit.test_orchestrator_auth tests.unit.test_orchestrator_client tests.unit.test_docker_orchestrator tests.unit.test_macos_orchestrator tests.unit.test_firecracker_orchestrator` (90 tests, all passed)
didericis-codex approved these changes 2026-07-25 21:44:29 -04:00
didericis-codex left a comment
Collaborator
Copy Link
Copy Source

Re-reviewed at e53104d. The previous blocking issue is resolved: the topology-based empty-key opt-out has been removed, and ControlPlaneProvisioning.orchestrator_key() now fails closed unconditionally. The corresponding unsafe test contract was removed and replaced with coverage for the unconditional failure.

I found no further actionable issues.

Verification:

  • git diff --check origin/main...HEAD
  • python3 -m unittest -v tests.unit.test_trust_domain tests.unit.test_orchestrator_auth tests.unit.test_orchestrator_client tests.unit.test_docker_orchestrator tests.unit.test_macos_orchestrator tests.unit.test_firecracker_orchestrator (90 tests, all passed)
Re-reviewed at e53104d. The previous blocking issue is resolved: the topology-based empty-key opt-out has been removed, and `ControlPlaneProvisioning.orchestrator_key()` now fails closed unconditionally. The corresponding unsafe test contract was removed and replaced with coverage for the unconditional failure. I found no further actionable issues. Verification: - `git diff --check origin/main...HEAD` - `python3 -m unittest -v tests.unit.test_trust_domain tests.unit.test_orchestrator_auth tests.unit.test_orchestrator_client tests.unit.test_docker_orchestrator tests.unit.test_macos_orchestrator tests.unit.test_firecracker_orchestrator` (90 tests, all passed)
didericis merged commit e53104d5c1 into main 2026-07-25 21:48:47 -04:00
Sign in to join this conversation.
Reviewers
No Reviewers
didericis-codex
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
Status/Needs Triage
Awaiting initial classification
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
didericis-claude (didericis (claude)) didericis-codex (didericis (codex)) didericis-gemma (gemma) didericis
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: didericis/bot-bottle#482
Delete Branch "didericis/refactor/control-plane-auth-provisioning-476"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?