starfleet/crew
Watch
1
0
Fork
You've already forked crew
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

feat: crew requires TLS material and verifies the fleet CA on outbound calls #9

Merged
troed merged 7 commits from feat/app-tier-tls into devel 2026-10-17 18:16:29 +02:00
Conversation 0 Commits 7 Files changed 19 +805 -62
troed commented 2026-10-17 18:08:23 +02:00
Owner
Copy link

feat: crew requires TLS material and verifies the fleet CA on outbound calls

What this is

Part of the fleet app-tier TLS change. Design and plan live in the
starfleet docs repo:

  • docs/superpowers/specs/2026-10-17-app-tier-tls-design.md
  • docs/superpowers/plans/2026-10-17-app-tier-tls.md
  • docs/superpowers/plans/2026-10-17-app-tier-tls-runbook.md (the live cutover)

Crew already served TLS in production (its live config sets both paths), but
the code made TLS optional: server_kwargs omitted the ssl kwargs unless both
paths were set, so an empty configuration silently served plain HTTP and a
partial one degraded differently. Meanwhile every outbound call — to
computer, to roster — was made with no verify argument, so it verified
against the system trust store, which does not contain the fleet's private CA.
computer has been TLS-required since the fleet-TLS-only change, so those calls
were already unverifiable.

This branch removes the silent fallback and gives every outbound client the
fleet CA.

Change

TLS is required for serving. server_kwargs(cfg) now raises ConfigError
naming tls_cert (both empty) or tls_key (partial) instead of omitting the
ssl kwargs. In production this is a no-op — crew already serves TLS — it closes
the silent path. serve() computes the kwargs before resolving the CA, so a
config missing both reports the primary error. Whitespace-only values are
stripped for the emptiness test, so " " produces the named error rather than a
uvicorn file error.

Outbound trust. New src/crew/trust.py::resolve_ca_bundle — byte-identical
to roster's apart from the docstring, deliberately, so the two services fail the
same way. Resolution order --ca-bundle → $STARFLEET_CA_BUNDLE →
[trust] ca_bundle → ConfigError naming all three, and the resolved bundle is
validated with load_verify_locations, so a missing path or a non-PEM file
fails at resolution naming the setting and the path rather than at the first
request. Threaded into the four call sites: _enroll_pcm,
_computer_get_devices, _computer_capture, and the roster provisioning client
(prompting/roster_client.py, which gained a ca_bundle and a shared _http()
helper).

The CLI can verify crew's own API. --ca-bundle is available to the client
subcommands (not just serve), so the documented
crew list --url https://<crew-host>:4030 --ca-bundle <ca.pem> works — crew is
TLS-only now, so that is the correct example. The default --url flipped to
https://127.0.0.1:4030. main() catches ConfigError for every command, so a
missing or unreadable CA prints a clean error: … and exits 1 rather than a
traceback.

Docs and examples. README admin-UI and CLI guidance corrected (they said
http://127.0.0.1:4030, which is wrong twice over — wrong scheme, and a host
that only works if the browser runs on the server); the TLS section documents
required material and the CA order; crew.toml.example gained [trust] and its
[computer] URLs now point at https://, because computer is TLS-required and
the file's own comment claims CA verification for those calls. makedoc.sh was
repaired — it could no longer start the server, since it generated a config with
no TLS material.

Verification

Sandbox suite at the branch head: 228 passed, ruff check /
ruff format --check / mypy src/ clean.

cd $HOME/dev/starfleet/crew
export UV_CACHE_DIR=/tmp/opencode/uv-cache      # ~/.cache/uv is read-only in the sandbox
uv sync --quiet                                 # host-made .venv is unusable in-sandbox
env -u ALL_PROXY -u all_proxy -u HTTPS_PROXY -u https_proxy -u HTTP_PROXY -u http_proxy \
  uv run ruff check src/ tests/ && uv run ruff format --check src/ tests/ \
  && uv run mypy src/ && uv run pytest tests/ -q

Five test_api_firmware.py failures appear without the env -u … prefix;
they are ALL_PROXY=socks5h demanding the absent socksio package, unrelated to
this change. Verification is proven by real TLS: a test stands up a self-signed
TLS peer and asserts the positive control (trusting the peer's cert succeeds)
against the negative (a different bundle refuses the handshake). The spy tests
put a different but valid bundle in the config tier and assert the explicit one
wins, so a dropped argument cannot be masked by the config fallback.

Tests that asserted plaintext behaviour were inverted, not deleted — the
no-TLS kwargs test now requires both paths, and both partial directions are
covered.

Cross-repo / ops

  • roster and communicator-esp32 carry the sibling pieces.
  • The live cutover is not part of this branch; it is the runbook in the
    starfleet docs repo. Note for the operator: crew serve resolves the CA
    eagerly at startup, so the redeploy is not a no-op — [trust] ca_bundle
    must exist first (the runbook says so).
  • tools/starfleet_trust.py (esp32) now also emits a roster leaf; adding files
    to DEV_FILES makes an existing shared .dev-trust incomplete, so the next
    ensure() regenerates it with a new CA and a flashed dev device needs a
    reflash.

Deferred minors (triaged ship)

  • A client command pointed at a plaintext --url still demands a resolvable CA
    (the plan's "all outbound clients", no scheme qualifier).
  • The API re-resolves and re-parses the CA bundle per outbound request; resolving
    once in create_app would do.
  • _recording_httpx_client is duplicated verbatim across three test files.
  • makedoc.sh's SAN probe greps 127\.0\.0\.1 as a substring, so 127.0.0.10
    would be accepted and then fail later as "the server never became healthy".
  • verify=<str> is deprecated in httpx 0.28.1 — a plan flaw (the plan
    mandated the string form). Verification is on today; a future httpx bump
    removes the form. Migration is to pass an ssl.SSLContext.
# feat: crew requires TLS material and verifies the fleet CA on outbound calls ## What this is Part of the fleet **app-tier TLS** change. Design and plan live in the `starfleet` docs repo: - `docs/superpowers/specs/2026-10-17-app-tier-tls-design.md` - `docs/superpowers/plans/2026-10-17-app-tier-tls.md` - `docs/superpowers/plans/2026-10-17-app-tier-tls-runbook.md` (the live cutover) Crew already *served* TLS in production (its live config sets both paths), but the code made TLS optional: `server_kwargs` omitted the ssl kwargs unless both paths were set, so an empty configuration silently served plain HTTP and a partial one degraded differently. Meanwhile every outbound call — to `computer`, to `roster` — was made with **no** `verify` argument, so it verified against the system trust store, which does not contain the fleet's private CA. `computer` has been TLS-required since the fleet-TLS-only change, so those calls were already unverifiable. This branch removes the silent fallback and gives every outbound client the fleet CA. ## Change **TLS is required for serving.** `server_kwargs(cfg)` now raises `ConfigError` naming `tls_cert` (both empty) or `tls_key` (partial) instead of omitting the ssl kwargs. In production this is a no-op — crew already serves TLS — it closes the silent path. `serve()` computes the kwargs **before** resolving the CA, so a config missing both reports the primary error. Whitespace-only values are stripped for the emptiness test, so `" "` produces the named error rather than a uvicorn file error. **Outbound trust.** New `src/crew/trust.py::resolve_ca_bundle` — byte-identical to roster's apart from the docstring, deliberately, so the two services fail the same way. Resolution order `--ca-bundle` → `$STARFLEET_CA_BUNDLE` → `[trust] ca_bundle` → `ConfigError` naming all three, and the resolved bundle is **validated** with `load_verify_locations`, so a missing path or a non-PEM file fails at resolution naming the setting and the path rather than at the first request. Threaded into the four call sites: `_enroll_pcm`, `_computer_get_devices`, `_computer_capture`, and the roster provisioning client (`prompting/roster_client.py`, which gained a `ca_bundle` and a shared `_http()` helper). **The CLI can verify crew's own API.** `--ca-bundle` is available to the client subcommands (not just `serve`), so the documented `crew list --url https://<crew-host>:4030 --ca-bundle <ca.pem>` works — crew is TLS-only now, so that is the correct example. The default `--url` flipped to `https://127.0.0.1:4030`. `main()` catches `ConfigError` for every command, so a missing or unreadable CA prints a clean `error: …` and exits 1 rather than a traceback. **Docs and examples.** README admin-UI and CLI guidance corrected (they said `http://127.0.0.1:4030`, which is wrong twice over — wrong scheme, and a host that only works if the browser runs on the server); the TLS section documents required material and the CA order; `crew.toml.example` gained `[trust]` and its `[computer]` URLs now point at `https://`, because computer is TLS-required and the file's own comment claims CA verification for those calls. `makedoc.sh` was repaired — it could no longer start the server, since it generated a config with no TLS material. ## Verification Sandbox suite at the branch head: **228 passed**, `ruff check` / `ruff format --check` / `mypy src/` clean. ```bash cd $HOME/dev/starfleet/crew export UV_CACHE_DIR=/tmp/opencode/uv-cache # ~/.cache/uv is read-only in the sandbox uv sync --quiet # host-made .venv is unusable in-sandbox env -u ALL_PROXY -u all_proxy -u HTTPS_PROXY -u https_proxy -u HTTP_PROXY -u http_proxy \ uv run ruff check src/ tests/ && uv run ruff format --check src/ tests/ \ && uv run mypy src/ && uv run pytest tests/ -q ``` Five `test_api_firmware.py` failures appear **without** the `env -u …` prefix; they are `ALL_PROXY=socks5h` demanding the absent `socksio` package, unrelated to this change. Verification is proven by real TLS: a test stands up a self-signed TLS peer and asserts the positive control (trusting the peer's cert succeeds) against the negative (a different bundle refuses the handshake). The spy tests put a *different but valid* bundle in the config tier and assert the explicit one wins, so a dropped argument cannot be masked by the config fallback. Tests that asserted plaintext behaviour were **inverted, not deleted** — the no-TLS kwargs test now requires both paths, and both partial directions are covered. ## Cross-repo / ops - `roster` and `communicator-esp32` carry the sibling pieces. - The live cutover is **not** part of this branch; it is the runbook in the `starfleet` docs repo. Note for the operator: `crew serve` resolves the CA eagerly at startup, so the redeploy is **not** a no-op — `[trust] ca_bundle` must exist first (the runbook says so). - `tools/starfleet_trust.py` (esp32) now also emits a `roster` leaf; adding files to `DEV_FILES` makes an existing shared `.dev-trust` incomplete, so the next `ensure()` regenerates it with a **new CA** and a flashed dev device needs a reflash. ## Deferred minors (triaged ship) - A client command pointed at a plaintext `--url` still demands a resolvable CA (the plan's "all outbound clients", no scheme qualifier). - The API re-resolves and re-parses the CA bundle per outbound request; resolving once in `create_app` would do. - `_recording_httpx_client` is duplicated verbatim across three test files. - `makedoc.sh`'s SAN probe greps `127\.0\.0\.1` as a substring, so `127.0.0.10` would be accepted and then fail later as "the server never became healthy". - `verify=<str>` is deprecated in httpx 0.28.1 — a **plan** flaw (the plan mandated the string form). Verification is on today; a future httpx bump removes the form. Migration is to pass an `ssl.SSLContext`. 
Also flip the CLI default --url to https://127.0.0.1:4030 so a bare crew list cannot fall back to plaintext; a new test pins it.
computer is TLS-required, so the example http:// enroll/devices/capture URLs could not work and contradicted the [trust] ca_bundle comment. test_ops now pins all three.
fix: app-tier TLS whole-branch review wave (crew)
All checks were successful
CI / Sanity check (ubuntu-latest) (pull_request) Successful in 37s
Details
1792b3d95d 
- main(): catch ConfigError so a missing/unreadable CA prints a clean
  message instead of a raw traceback.
- serve(): validate the TLS pair before resolving the CA, so a config
  missing both reports the primary TLS error, not the CA error.
- server_kwargs(): strip before the emptiness test so a whitespace-only
  tls_cert/tls_key is named here rather than failing inside uvicorn.
- tests: match the distinguishing tail of the refusal message, add the
  reverse partial direction and a whitespace-only case, add a clean-error
  test and a TLS-before-CA test; pin the example's [trust] ca_bundle key
  by parsing the TOML (a substring check could not catch it going away).
- README: add crew->roster to the outbound-CA enumeration and scope the
  no---url sentence to the client commands (doctor never hits the network).
- makedoc.sh: crew serves TLS only now, so the generated config points at
  the shared dev trust set (crew.pem/crew.key/ca.pem) and declares
  [trust] ca_bundle; it fails with a clear message when the set is absent
  or its leaf does not cover loopback. makedoc-capture.mjs lets the local
  dev certificate load.
troed merged commit 153c2a4239 into devel 2026-10-17 18:16:29 +02:00
troed deleted branch feat/app-tier-tls 2026-10-17 18:16:29 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
starfleet/crew!9
No description provided.