From d500563963db8cead5293a2278ca0f6961746c90 Mon Sep 17 00:00:00 2001 From: mptyl Date: Sun, 12 Jul 2026 11:33:13 +0200 Subject: [PATCH] fix(security): scrub deployment secrets from Pi child --- .superpowers/sdd/task-3-report.md | 129 ++++------------------ backend/src/pi/provider-credentials.ts | 13 +++ backend/test/provider-credentials.test.ts | 20 +++- 3 files changed, 51 insertions(+), 111 deletions(-) diff --git a/.superpowers/sdd/task-3-report.md b/.superpowers/sdd/task-3-report.md index c1e9b183..eafc1d7d 100644 --- a/.superpowers/sdd/task-3-report.md +++ b/.superpowers/sdd/task-3-report.md @@ -1,103 +1,4 @@ -# Task 3 report — vector port and wrappers - -## Status - -Complete. Added the transport-neutral vector port, HTTP and legacy-direct wrappers, and -routed the existing `RestSearcher` and `DirectSearcher` through them while retaining the -canonical `VectorRecord` and `VectorHit` models. - -## TDD evidence - -- RED: `cd harness && .venv/bin/pytest tests/test_vector_port_contract.py -q` - - Result: collection failed with `ModuleNotFoundError: No module named - 'tht.adapters.vector'` (expected missing-port failure). -- GREEN: `cd harness && .venv/bin/pytest tests/test_vector_port_contract.py -q` - - Result: `4 passed in 0.11s`. - -## Verification - -- Required regression command: - `cd harness && .venv/bin/pytest tests/test_vector_port_contract.py - tests/test_vector_dual_key.py tests/test_search_similar_kinds.py - tests/test_memory_save_one.py tests/test_solved_question.py -q` - - Result: `26 passed in 0.12s` (fresh final run; earlier run: 26 passed in 0.16s). -- Broader vector-focused regression: - `cd harness && .venv/bin/pytest tests -q -k 'vector or search_similar or - memory_save_one or solved_question'` - - Result: `29 passed, 370 deselected in 0.49s`. -- Scoped lint: - `cd harness && .venv/bin/ruff check tht/ports/vector.py tht/adapters/vector - tht/vectorstore/reader.py tests/test_vector_port_contract.py` - - Result: `All checks passed!`. -- Whitespace check: `git diff --check` - - Result: exit 0, no output. -- Staged whitespace check: `git diff --cached --check` - - Result: exit 0, no output. - -## Commit - -`ff4d662 refactor(vector): define store contract` - -Only the six Task 3 implementation/test files were included in the commit. - -## Self-review / concerns - -- Reader and writer clients remain separate: search and health use only the reader; - hashes and upsert use only the writer. -- A missing writer reports `upsert=False` and raises `VectorWriteUnavailable`, including - for an empty upsert, so deployments cannot silently bypass the write credential gate. -- Existing REST kind forwarding, legacy-404 fallback, post-filtering, global similarity - merge, and direct table-per-kind behavior remain delegated to their established code. -- The port re-exports existing vector models instead of duplicating result types. -- Non-blocking design constraint: embedded values for the new generic `upsert` contract are - carried in `VectorRecord.metadata['embedding']`; this preserves the existing canonical - record model and REST payload without changing out-of-scope `records.py`. A later direct - pgvector implementation may choose to formalize that field across adapters. - -## Authorized contract correction - -Status: complete. Commit: `fe8d70d fix(vector): separate write transport fields`. - -The earlier metadata-envelope concern above is superseded. The contract now exports a -dedicated frozen `VectorWriteRecord` containing the canonical `VectorRecord`, precomputed -embedding, and content hash. `VectorStore.upsert` accepts only that envelope. HTTP row -serialization takes transport fields from the envelope and preserves `record.metadata` -unchanged, including legitimate metadata keys named `embedding` and `content_hash`. - -### Corrective TDD evidence - -- RED: `cd harness && .venv/bin/pytest tests/test_vector_port_contract.py -q` - - Result: collection failed with `ImportError: cannot import name 'VectorWriteRecord' from - 'tht.ports.vector'` (expected missing-envelope failure). -- GREEN: `cd harness && .venv/bin/pytest tests/test_vector_port_contract.py -q` - - Result: `7 passed in 0.11s`. -- Required regression suite: - `cd harness && .venv/bin/pytest tests/test_vector_port_contract.py - tests/test_vector_dual_key.py tests/test_search_similar_kinds.py - tests/test_memory_save_one.py tests/test_solved_question.py -q` - - Result: `29 passed in 0.15s` (fresh final run; earlier run: 29 passed in 0.14s). -- Scoped lint: - `cd harness && .venv/bin/ruff check tht/ports/__init__.py tht/ports/vector.py - tht/adapters/vector tht/vectorstore/reader.py tests/test_vector_port_contract.py` - - Result: `All checks passed!`. -- Whitespace check: `git diff --check` - - Result: exit 0, no output. - -### Corrective self-review - -- A real `evidence_records(...)` canonical builder record is serialized in the contract tests. -- Collision coverage proves semantic `embedding` and `content_hash` metadata survive while - distinct envelope values occupy the RPC row's top-level transport fields. -- Reader health/search still use only the reader client; hashes/upsert still use only writer. -- Existing kind forwarding, legacy 404 fallback, post-filtering, similarity merge, and direct - search behavior remain unchanged and covered by the required regression suite. -- The tracked plan, regenerated Task 3 scratch brief, port exports, adapter signature, and this - report now consistently describe `VectorWriteRecord`. -- Concerns: none known. - ---- - -# Task simple-config-3 report — one bundle for local-vector and preprocess +# Task 3 report — one secret bundle for local services ## Status @@ -113,8 +14,8 @@ to the harness and materializes short-lived 0600 password files for workspace re `vector_reader_password` Compose secret declaration. - GREEN: the same command passes after the bundle conversion and verifies local-vector workspace interpolation and shared secret mounts. -- Added bundle parser regressions to `./scripts/test-vector-secret-policy.sh`: comments/blank - lines are accepted and an unrelated duplicate key is rejected. +- `./scripts/test-vector-secret-policy.sh` covers comments/blank lines and rejects an + unrelated duplicate key. ## Verification @@ -123,17 +24,25 @@ to the harness and materializes short-lived 0600 password files for workspace re - `./scripts/test-vector-backup-restore-safety.sh` — passed. - `./scripts/test-default-compose.sh` — passed. - `./scripts/test-container-deployment.sh` — passed. -- `./scripts/local-vector-smoke.sh` — passed (real Docker; bootstrap rotation, role +- `./scripts/local-vector-smoke.sh` — passed with real Docker (bootstrap rotation, role reconciliation, migration, persistence and restart). -- `./scripts/preprocess-smoke.sh` — passed (real Docker; unchanged rerun, mutation, DWH job, - ACTIVE publication and cleanup). +- `./scripts/preprocess-smoke.sh` — passed with real Docker (unchanged rerun, mutation, DWH + job, ACTIVE publication and cleanup). +- `./scripts/preprocess-smoke.sh --cleanup-failure` — passed. - `git diff --check` and `sh -n` gates — passed. -## Commit +## Critical review fix -Pending: `feat(compose): use one secret bundle for local services`. +`buildPiChildEnv` now removes `THT_DWH_API_KEY`, `THT_VEC_API_KEY`, `THT_VEC_WRITE_API_KEY`, +`THT_SSL_CA`, `THT_CA`, and their file metadata before spawning Pi. A regression test proves +that neither secret values nor bundle/file metadata are inherited by the Pi child. -## Concerns +## Commits -The rotation helper still accepts old/new scratch files because that is its explicit CLI -contract; the smoke script keeps those files outside Compose and mounts only the bundle. +- `70a19f2 feat(compose): use one secret bundle for local services` +- pending follow-up: scrub DWH/vector/CA credentials from Pi child environment. + +## Concern + +The rotation helper retains its old/new scratch-file CLI contract; smoke tests keep those files +outside Compose and mount only the bundle. diff --git a/backend/src/pi/provider-credentials.ts b/backend/src/pi/provider-credentials.ts index ba9d06b0..91e7f607 100644 --- a/backend/src/pi/provider-credentials.ts +++ b/backend/src/pi/provider-credentials.ts @@ -107,6 +107,11 @@ export function buildPiChildEnv(opts: { const env = { ...(opts.ambient ?? process.env), ...opts.additions }; delete env.PI_PROVIDER_API_KEY; delete env.THT_SECRETS_FILE; + delete env.THT_DWH_API_KEY; + delete env.THT_VEC_API_KEY; + delete env.THT_VEC_WRITE_API_KEY; + delete env.THT_SSL_CA; + delete env.THT_CA; delete env.THT_MODEL_API_KEY_FILE; delete env.THT_DWH_API_KEY_SECRET_FILE; delete env.THT_VEC_API_KEY_SECRET_FILE; @@ -116,6 +121,14 @@ export function buildPiChildEnv(opts: { delete env.THT_VECTOR_MIGRATOR_PASSWORD_SECRET_FILE; delete env.THT_VECTOR_READER_PASSWORD_SECRET_FILE; delete env.THT_VECTOR_WRITER_PASSWORD_SECRET_FILE; + delete env.THT_VECTOR_BOOTSTRAP_PASSWORD_FILE; + delete env.THT_VECTOR_MIGRATOR_PASSWORD_FILE; + delete env.THT_VECTOR_READER_PASSWORD_FILE; + delete env.THT_VECTOR_WRITER_PASSWORD_FILE; + delete env.THT_DWH_API_KEY_FILE; + delete env.THT_VEC_API_KEY_FILE; + delete env.THT_VEC_WRITE_API_KEY_FILE; + delete env.THT_SSL_CA_FILE; for (const name of PI_0803_CREDENTIAL_ENV_NAMES) delete env[name]; const provider = canonicalPiProvider(opts.provider); if (provider && COMPOUND_PROVIDERS.has(provider)) { diff --git a/backend/test/provider-credentials.test.ts b/backend/test/provider-credentials.test.ts index 458fdbc2..3210e944 100644 --- a/backend/test/provider-credentials.test.ts +++ b/backend/test/provider-credentials.test.ts @@ -118,10 +118,28 @@ test("single-key providers scrub ambient compound companions before injecting th test("bundle value is injected without exposing bundle metadata to Pi", () => { const env = buildPiChildEnv({ - ambient: { THT_SECRETS_FILE: "/run/secrets/thothii.secrets", THT_MODEL_API_KEY_FILE: "/run/secrets/model" }, + ambient: { + THT_SECRETS_FILE: "/run/secrets/thothii.secrets", THT_MODEL_API_KEY_FILE: "/run/secrets/model", + THT_DWH_API_KEY: "dwh-secret", THT_VEC_API_KEY: "vec-reader-secret", + THT_VEC_WRITE_API_KEY: "vec-writer-secret", THT_SSL_CA: "/run/secrets/ca.pem", + THT_CA: "/run/secrets/ca.pem", THT_DWH_API_KEY_SECRET_FILE: "/run/secrets/dwh", + THT_VECTOR_READER_PASSWORD_FILE: "/tmp/thothii-secrets/reader", + THT_VECTOR_WRITER_PASSWORD_FILE: "/tmp/thothii-secrets/writer", + THT_DWH_API_KEY_FILE: "/run/secrets/dwh", THT_VEC_API_KEY_FILE: "/run/secrets/vec", + }, provider: "openai", credentialValue: "bundle-secret", }); expect(env.OPENAI_API_KEY).toBe("bundle-secret"); expect(env).not.toHaveProperty("THT_SECRETS_FILE"); expect(env).not.toHaveProperty("THT_MODEL_API_KEY_FILE"); + expect(env).not.toHaveProperty("THT_DWH_API_KEY"); + expect(env).not.toHaveProperty("THT_VEC_API_KEY"); + expect(env).not.toHaveProperty("THT_VEC_WRITE_API_KEY"); + expect(env).not.toHaveProperty("THT_SSL_CA"); + expect(env).not.toHaveProperty("THT_CA"); + expect(env).not.toHaveProperty("THT_DWH_API_KEY_SECRET_FILE"); + expect(env).not.toHaveProperty("THT_VECTOR_READER_PASSWORD_FILE"); + expect(env).not.toHaveProperty("THT_VECTOR_WRITER_PASSWORD_FILE"); + expect(env).not.toHaveProperty("THT_DWH_API_KEY_FILE"); + expect(env).not.toHaveProperty("THT_VEC_API_KEY_FILE"); });