Compare commits
3 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 350b1def0d | |||
| c40d359b3d | |||
| 46e595ffb1 |
@@ -0,0 +1,128 @@
|
|||||||
|
# PRD prd-new: Encrypted at-rest egress secrets (SecretProvider, interim slice)
|
||||||
|
|
||||||
|
- **Status:** Draft
|
||||||
|
- **Author:** didericis
|
||||||
|
- **Created:** 2026-07-21
|
||||||
|
- **Issue:** #355
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
An interim step toward the generic `SecretProvider` (#355) that stops short
|
||||||
|
of per-request minting. Today the orchestrator holds each bottle's egress
|
||||||
|
auth tokens **in process memory only**, so any infra-container recreation
|
||||||
|
silently strips every already-running bottle of its upstream credentials.
|
||||||
|
This PRD makes those secrets survive a gateway restart by persisting them
|
||||||
|
**encrypted**, under a key that is not itself sitting next to the
|
||||||
|
ciphertext.
|
||||||
|
|
||||||
|
The end state in #355 — short-lived, scoped credentials minted per request
|
||||||
|
— removes the need to store anything durable at all. That is a larger
|
||||||
|
change gated on per-upstream minting support. This slice buys back
|
||||||
|
restart-survivability now without regressing to plaintext secrets at rest.
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
`Orchestrator._tokens` (`bot_bottle/orchestrator/service.py:74-79`) is a
|
||||||
|
plain in-memory dict, deliberately never written to the registry DB:
|
||||||
|
|
||||||
|
> Held **in memory only** — never written to the registry DB — so the
|
||||||
|
> gateway can inject each bottle's upstream credential without secrets at
|
||||||
|
> rest. Lost on restart (re-launch re-registers them); the future
|
||||||
|
> SecretProvider (#355) replaces this with per-request minting.
|
||||||
|
|
||||||
|
The registry itself *is* durable (SQLite on a container-only volume), and
|
||||||
|
so is the gateway CA since #450 / `2cd44cf7`. The tokens are now the only
|
||||||
|
piece of gateway state that does not survive a restart, which makes the
|
||||||
|
failure mode both silent and confusing.
|
||||||
|
|
||||||
|
### Observed failure
|
||||||
|
|
||||||
|
Checking out a branch that touches `bot_bottle/**/*.py` changes
|
||||||
|
`source_hash()` (`bot_bottle/orchestrator/lifecycle.py:88-99`).
|
||||||
|
`MacosInfraService._source_current()`
|
||||||
|
(`bot_bottle/backend/macos_container/infra.py:159-169`) sees the mismatch
|
||||||
|
and `ensure_running()` force-removes and recreates the infra container
|
||||||
|
(`infra.py:198-208`). The registry rows survive on the DB volume; the CA
|
||||||
|
survives on its host bind-mount; `_tokens` comes back empty.
|
||||||
|
|
||||||
|
Every already-running bottle then fails closed, mid-session, on its next
|
||||||
|
outbound request:
|
||||||
|
|
||||||
|
- `/resolve` succeeds — the bottle is still `active` in
|
||||||
|
`orchestrator_bottles` and its policy blob is served intact, including
|
||||||
|
`- host: "api.anthropic.com"` with `auth_scheme: Bearer` /
|
||||||
|
`token_env: EGRESS_TOKEN_0`.
|
||||||
|
- `tokens_for()` returns `{}`, so the resolved env overlay has no
|
||||||
|
`EGRESS_TOKEN_0`.
|
||||||
|
- `decide()` (`bot_bottle/egress_addon_core.py:644-652`) blocks with
|
||||||
|
`egress: route for 'api.anthropic.com' declared auth but env var
|
||||||
|
'EGRESS_TOKEN_0' is unset` — an 89-byte `403` on every request.
|
||||||
|
|
||||||
|
Confirmed live on the macOS backend on 2026-07-21: two bottles running
|
||||||
|
since 20:29/20:30 were still registered `active` with valid policy after
|
||||||
|
the 23:05 infra recreation, and both took 89-byte `403`s from then on,
|
||||||
|
while a bottle launched *after* the recreation egressed normally. The
|
||||||
|
recovery today is to relaunch every affected bottle.
|
||||||
|
|
||||||
|
Note this is a re-attachment blocker distinct from #443/#445 and from #450
|
||||||
|
— the CA and the gateway address were both fine. It is specifically the
|
||||||
|
credential wipe.
|
||||||
|
|
||||||
|
## Goals / Success criteria
|
||||||
|
|
||||||
|
1. A bottle's egress auth tokens survive infra-container recreation: an
|
||||||
|
already-running bottle keeps egressing across a gateway restart with no
|
||||||
|
relaunch and no operator action.
|
||||||
|
2. Secrets are **never** at rest in plaintext, and never at rest next to a
|
||||||
|
key that trivially decrypts them.
|
||||||
|
3. Compromise of the registry DB file alone does not yield usable
|
||||||
|
upstream credentials.
|
||||||
|
4. The stored form is revocable and rotatable without relaunching bottles
|
||||||
|
that are not affected.
|
||||||
|
5. Reap/teardown destroys a bottle's stored secrets along with its
|
||||||
|
registry row (no ciphertext outliving its bottle).
|
||||||
|
6. Migration is transparent: existing bottles keep working, no manifest
|
||||||
|
changes required.
|
||||||
|
|
||||||
|
## Non-goals
|
||||||
|
|
||||||
|
- **Per-request minting** of short-lived scoped credentials. That is the
|
||||||
|
#355 end state; this PRD is explicitly the interim slice and should not
|
||||||
|
foreclose it.
|
||||||
|
- Generalizing `DeployKeyProvisioner` into the full `SecretProvider` ABC,
|
||||||
|
or the manifest-level `{ provider: <name> }` reference surface.
|
||||||
|
- User-extensible provider discovery (`~/.bot-bottle/contrib/<name>/`).
|
||||||
|
- Changing the `/resolve` contract's shape (it already carries `tokens`).
|
||||||
|
- Fixing the *trigger* — `source_hash` churn on branch switch. Recreating
|
||||||
|
infra is legitimate; it just must not cost running bottles their
|
||||||
|
credentials. A separate guard that refuses recreation while bottles are
|
||||||
|
active is complementary and out of scope here.
|
||||||
|
|
||||||
|
## Design
|
||||||
|
|
||||||
|
> **TODO (didericis):** the encryption flow goes here — key custody, where
|
||||||
|
> the key material lives relative to the ciphertext, the wrap/unwrap path
|
||||||
|
> at register and at `/resolve`, and what an attacker who holds only the
|
||||||
|
> DB (or only the host, or only the infra container) can recover.
|
||||||
|
|
||||||
|
Constraints the design has to satisfy, for reference while drafting:
|
||||||
|
|
||||||
|
- The gateway's `PolicyResolver` needs the cleartext at request time, on
|
||||||
|
the data-plane path, so unwrap has to be cheap enough to sit in a
|
||||||
|
per-flow `/resolve` (or be cached in memory after first unwrap).
|
||||||
|
- The infra container is recreated routinely and unattended. Anything
|
||||||
|
requiring an interactive unlock on every recreation defeats the goal.
|
||||||
|
- The DB lives on a container-only volume that the host does not mount, so
|
||||||
|
host-side and guest-side components see different filesystems — that
|
||||||
|
asymmetry is available as a place to split custody.
|
||||||
|
- The agent must never be able to reach the key material. It is a separate
|
||||||
|
container with no control-plane token, which is the existing boundary.
|
||||||
|
|
||||||
|
## Open questions
|
||||||
|
|
||||||
|
- Where does the unwrap key live, and what recreates/re-derives it when the
|
||||||
|
infra container is rebuilt?
|
||||||
|
- Is the cleartext cached in memory after first unwrap, or unwrapped per
|
||||||
|
request? (Latency vs. exposure window.)
|
||||||
|
- What is the rotation story — re-wrap in place, or force re-registration?
|
||||||
|
- Does this land behind a flag, or replace `_tokens` outright?
|
||||||
@@ -173,6 +173,58 @@ if __name__ == "__main__":
|
|||||||
unittest.main()
|
unittest.main()
|
||||||
|
|
||||||
|
|
||||||
|
class TestAgentSecrets(unittest.TestCase):
|
||||||
|
"""store/get/delete for the bottled_agent_secrets table."""
|
||||||
|
|
||||||
|
def setUp(self) -> None:
|
||||||
|
self._tmp = tempfile.TemporaryDirectory()
|
||||||
|
self.db = Path(self._tmp.name) / "registry.db"
|
||||||
|
self.store = RegistryStore(self.db)
|
||||||
|
self.store.migrate()
|
||||||
|
|
||||||
|
def tearDown(self) -> None:
|
||||||
|
self._tmp.cleanup()
|
||||||
|
|
||||||
|
def test_store_and_get_roundtrip(self) -> None:
|
||||||
|
self.store.store_agent_secrets("bottle-1", {"EGRESS_TOKEN_1": "enc-val-a"})
|
||||||
|
got = self.store.get_agent_secrets("bottle-1")
|
||||||
|
self.assertEqual({"EGRESS_TOKEN_1": "enc-val-a"}, got)
|
||||||
|
|
||||||
|
def test_get_returns_empty_when_none_stored(self) -> None:
|
||||||
|
self.assertEqual({}, self.store.get_agent_secrets("no-such-bottle"))
|
||||||
|
|
||||||
|
def test_store_replaces_existing_rows(self) -> None:
|
||||||
|
self.store.store_agent_secrets("bottle-1", {"K": "old"})
|
||||||
|
self.store.store_agent_secrets("bottle-1", {"K": "new", "K2": "v2"})
|
||||||
|
got = self.store.get_agent_secrets("bottle-1")
|
||||||
|
self.assertEqual({"K": "new", "K2": "v2"}, got)
|
||||||
|
|
||||||
|
def test_delete_removes_secrets(self) -> None:
|
||||||
|
self.store.store_agent_secrets("bottle-1", {"K": "v"})
|
||||||
|
self.store.delete_agent_secrets("bottle-1")
|
||||||
|
self.assertEqual({}, self.store.get_agent_secrets("bottle-1"))
|
||||||
|
|
||||||
|
def test_delete_is_idempotent_on_missing(self) -> None:
|
||||||
|
self.store.delete_agent_secrets("no-such-bottle") # must not raise
|
||||||
|
|
||||||
|
def test_secrets_are_isolated_by_bottle_id(self) -> None:
|
||||||
|
self.store.store_agent_secrets("bottle-1", {"K": "for-1"})
|
||||||
|
self.store.store_agent_secrets("bottle-2", {"K": "for-2"})
|
||||||
|
self.assertEqual({"K": "for-1"}, self.store.get_agent_secrets("bottle-1"))
|
||||||
|
self.assertEqual({"K": "for-2"}, self.store.get_agent_secrets("bottle-2"))
|
||||||
|
|
||||||
|
def test_secrets_isolated_by_type(self) -> None:
|
||||||
|
self.store.store_agent_secrets("bottle-1", {"K": "injected"}, secret_type="injected_env_var")
|
||||||
|
self.store.store_agent_secrets("bottle-1", {"K": "other"}, secret_type="other_type")
|
||||||
|
self.assertEqual({"K": "injected"}, self.store.get_agent_secrets("bottle-1"))
|
||||||
|
self.assertEqual({"K": "other"}, self.store.get_agent_secrets("bottle-1", secret_type="other_type"))
|
||||||
|
|
||||||
|
def test_secrets_persist_across_reopen(self) -> None:
|
||||||
|
self.store.store_agent_secrets("bottle-1", {"K": "v"})
|
||||||
|
reopened = RegistryStore(self.db)
|
||||||
|
self.assertEqual({"K": "v"}, reopened.get_agent_secrets("bottle-1"))
|
||||||
|
|
||||||
|
|
||||||
class TestReapAbsent(unittest.TestCase):
|
class TestReapAbsent(unittest.TestCase):
|
||||||
"""`reap_absent` — the self-heal for rows whose bottle is gone.
|
"""`reap_absent` — the self-heal for rows whose bottle is gone.
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,96 @@
|
|||||||
|
"""Unit tests for per-bottle egress secret encryption (PRD prd-new-secret-provider)."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import unittest
|
||||||
|
|
||||||
|
from bot_bottle.orchestrator.secret_store import (
|
||||||
|
ENV_VAR_SECRET_NAME,
|
||||||
|
decrypt_value,
|
||||||
|
encrypt_value,
|
||||||
|
new_env_var_secret,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
class TestNewEnvVarSecret(unittest.TestCase):
|
||||||
|
def test_returns_non_empty_string(self) -> None:
|
||||||
|
s = new_env_var_secret()
|
||||||
|
self.assertIsInstance(s, str)
|
||||||
|
self.assertTrue(len(s) > 0)
|
||||||
|
|
||||||
|
def test_secrets_are_unique(self) -> None:
|
||||||
|
keys = {new_env_var_secret() for _ in range(50)}
|
||||||
|
self.assertEqual(50, len(keys))
|
||||||
|
|
||||||
|
def test_no_padding_characters(self) -> None:
|
||||||
|
# URL-safe base64, padding stripped — should round-trip cleanly
|
||||||
|
for _ in range(20):
|
||||||
|
self.assertNotIn("=", new_env_var_secret())
|
||||||
|
|
||||||
|
|
||||||
|
class TestEncryptDecryptRoundtrip(unittest.TestCase):
|
||||||
|
def setUp(self) -> None:
|
||||||
|
self.secret = new_env_var_secret()
|
||||||
|
|
||||||
|
def _rt(self, plaintext: str) -> str:
|
||||||
|
return decrypt_value(self.secret, encrypt_value(self.secret, plaintext))
|
||||||
|
|
||||||
|
def test_roundtrip_short_value(self) -> None:
|
||||||
|
self.assertEqual("sk-abc123", self._rt("sk-abc123"))
|
||||||
|
|
||||||
|
def test_roundtrip_empty_string(self) -> None:
|
||||||
|
self.assertEqual("", self._rt(""))
|
||||||
|
|
||||||
|
def test_roundtrip_long_value_crosses_block_boundary(self) -> None:
|
||||||
|
# 32 bytes is exactly one HMAC-SHA256 block; 65 bytes crosses two.
|
||||||
|
plaintext = "x" * 65
|
||||||
|
self.assertEqual(plaintext, self._rt(plaintext))
|
||||||
|
|
||||||
|
def test_roundtrip_unicode(self) -> None:
|
||||||
|
self.assertEqual("héllo wörld", self._rt("héllo wörld"))
|
||||||
|
|
||||||
|
def test_encrypt_produces_different_ciphertexts_each_call(self) -> None:
|
||||||
|
ct1 = encrypt_value(self.secret, "same")
|
||||||
|
ct2 = encrypt_value(self.secret, "same")
|
||||||
|
self.assertNotEqual(ct1, ct2) # fresh nonce each call
|
||||||
|
|
||||||
|
def test_ciphertext_is_url_safe_base64(self) -> None:
|
||||||
|
ct = encrypt_value(self.secret, "hello")
|
||||||
|
# no '+', '/', '=' — URL-safe and padding-stripped
|
||||||
|
for ch in ("+", "/", "="):
|
||||||
|
self.assertNotIn(ch, ct)
|
||||||
|
|
||||||
|
|
||||||
|
class TestDecryptErrors(unittest.TestCase):
|
||||||
|
def setUp(self) -> None:
|
||||||
|
self.secret = new_env_var_secret()
|
||||||
|
|
||||||
|
def test_wrong_key_raises_value_error(self) -> None:
|
||||||
|
ct = encrypt_value(self.secret, "secret-token")
|
||||||
|
other_key = new_env_var_secret()
|
||||||
|
# Wrong key produces garbage bytes; decrypt_value raises ValueError
|
||||||
|
# when the result is non-UTF-8 (which is very likely for 12-char data).
|
||||||
|
# We allow it to succeed only if garbage happens to be valid UTF-8, but
|
||||||
|
# the plaintext must not match.
|
||||||
|
try:
|
||||||
|
result = decrypt_value(other_key, ct)
|
||||||
|
self.assertNotEqual("secret-token", result)
|
||||||
|
except ValueError:
|
||||||
|
pass
|
||||||
|
|
||||||
|
def test_truncated_blob_raises_value_error(self) -> None:
|
||||||
|
with self.assertRaises(ValueError):
|
||||||
|
decrypt_value(self.secret, "dG9vc2hvcnQ") # "tooshort" — under 16 nonce bytes
|
||||||
|
|
||||||
|
def test_invalid_base64_raises_value_error(self) -> None:
|
||||||
|
with self.assertRaises(ValueError):
|
||||||
|
decrypt_value(self.secret, "!!not-base64!!")
|
||||||
|
|
||||||
|
|
||||||
|
class TestConstant(unittest.TestCase):
|
||||||
|
def test_env_var_secret_name(self) -> None:
|
||||||
|
self.assertEqual("ENV_VAR_SECRET", ENV_VAR_SECRET_NAME)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
@@ -13,6 +13,7 @@ from unittest.mock import patch
|
|||||||
|
|
||||||
from bot_bottle.orchestrator.broker import LaunchBroker, LaunchRequest, StubBroker
|
from bot_bottle.orchestrator.broker import LaunchBroker, LaunchRequest, StubBroker
|
||||||
from bot_bottle.orchestrator.registry import RegistryStore
|
from bot_bottle.orchestrator.registry import RegistryStore
|
||||||
|
from bot_bottle.orchestrator.secret_store import new_env_var_secret
|
||||||
from bot_bottle.orchestrator.service import Orchestrator
|
from bot_bottle.orchestrator.service import Orchestrator
|
||||||
from bot_bottle.orchestrator.gateway import Gateway
|
from bot_bottle.orchestrator.gateway import Gateway
|
||||||
from bot_bottle.store_manager import StoreManager
|
from bot_bottle.store_manager import StoreManager
|
||||||
|
|||||||
Reference in New Issue
Block a user