fix(security): scrub deployment secrets from Pi child
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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)) {
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user