Ben Stull
9e6756f678
Backfills transcripts that existed only locally in ohm-infra: - 0019.1/.2/.3 - UX-polish wave subagent transcripts (driver 0019.0 never finalized) - 0026.1-.9 - security-audit-0026 subagent transcripts (driver abandoned/closed-out by 0068; audit drove published 0030 remediation) - 0073.0 - PPE/progressive-delivery + engineering-handbook session, originally drafted as a duplicate 0046.0; reassigned next free number (0072 taken by a concurrent session) sessions.json: add 0019/0073 titles, update 0026 title. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
77 lines
3.4 KiB
Markdown
77 lines
3.4 KiB
Markdown
# SESSION-0026.9 — flotilla audit finding L1 remediation
|
|
|
|
Parent: SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md
|
|
|
|
Mode: REMEDIATION (operator authorized). Subsession of OHM session 0026.
|
|
Scope: fix flotilla audit finding L1 in
|
|
`ohm-rfc-app-flotilla/ohm_rfc_app_flotilla/deploy.py` + a test.
|
|
No commit, no push, no VERSION bump, no VM/deploy actions.
|
|
|
|
## Finding L1 (recap)
|
|
|
|
`_PhaseRunner.run` persisted each phase's `detail` (which can embed a
|
|
failed command's `stderr[:240]`) into the `deploys.phases` JSON column
|
|
via `deploy_log.append_phase` WITHOUT redaction. Secret redaction
|
|
(`_ResolvedSecrets.redact_live`) was applied only to the separate
|
|
`error_summary` field in `_finalize_failure`. A secret surfacing in a
|
|
command's stderr would land unredacted in flotilla state — a §3
|
|
invariant 1 / §10.2 leak.
|
|
|
|
## Design choice — how the redactor was threaded
|
|
|
|
- Added an OPTIONAL field `redactor: _ResolvedSecrets | None = None` to
|
|
the `_PhaseRunner` dataclass, plus a small `_redact(text)` helper that
|
|
calls `redactor.redact_live(text)` when the redactor is present and
|
|
falls back to the raw text when it's None.
|
|
- Chose OPTIONAL over REQUIRED because it's the lower-churn option:
|
|
it preserves the ability to construct a `_PhaseRunner` without a
|
|
resolved-secrets context (none of the current tests do, but the
|
|
nullable style matches the rest of the module's injected-dependency
|
|
shape), and required no signature changes at any other call site.
|
|
- `run_deploy` constructs the only production runner and now passes
|
|
`redactor=resolved`. Phases run inside the `_ResolvedSecrets`
|
|
`with`/try scope, so the bytearrays are still live when `_redact`
|
|
runs during the phase — redaction happens before `scrub()` zeroes
|
|
them at scope exit, exactly as the finding requires.
|
|
- Both the success-path detail and the failure-path detail are now
|
|
redacted before they reach the `PhaseRecord` (persisted JSON) AND
|
|
before the `on_progress(...)` strings that interpolate the detail
|
|
(those can reach operator terminals/logs).
|
|
- `_finalize_failure` still redacts the summary it builds from the
|
|
last phase's detail; since that detail is now already redacted, the
|
|
second pass is idempotent (no behavior change).
|
|
|
|
## Diff summary
|
|
|
|
deploy.py:
|
|
- `_PhaseRunner`: new `redactor` field + `_redact` helper; `run`
|
|
computes `safe_detail = self._redact(...)` on both paths and uses it
|
|
for the `PhaseRecord.detail`, the `on_progress` FAILED message, and
|
|
the returned detail.
|
|
- `run_deploy`: `_PhaseRunner(...)` construction now passes
|
|
`redactor=resolved`.
|
|
|
|
tests/test_deploy.py:
|
|
- Extended `test_run_deploy_redacts_secret_bytes_from_error_summary`
|
|
to ALSO load the persisted row and assert the failing phase's
|
|
`detail` contains `[REDACTED]` and not the stub secret, and that no
|
|
phase detail anywhere carries the raw stub bytes.
|
|
|
|
## Test added / extended
|
|
|
|
Reused the existing redaction test (which drives a real `run_deploy`
|
|
with a `LeakyFail` ssh stub that vomits a fake secret into stderr) and
|
|
added the `deploys.phases` assertions. Stub value is the module's
|
|
`STUB_SECRET_BYTES = b"NEVER-LEAK-stub-secret-payload"` — a fake, never
|
|
a real secret.
|
|
|
|
## Test-run result
|
|
|
|
`.venv/bin/pytest tests/ -q` → 157 passed in 0.28s. Suite was green
|
|
before the change (full-suite run, not a known-broken baseline).
|
|
|
|
## Cut state
|
|
|
|
Code change + test in place. No commit, no push, no VERSION bump, no
|
|
VM/deploy action. Driver handles release.
|