tegwick
0d62ac501d
docs/review-optimize-checklist.md: six checks (naming, TTL/scoping,
redundancy, compaction, ease of use, posture), applied for real to the
rein-openweights plan's section 4 -- including a genuinely useful
finding (credentials.py already expects this exact path/delivery shape,
zero code changes needed to consume it).
docs/executive-summary-format.md: six fixed fields, no bao syntax, no
restating earlier sections, explicit approve/reject/revise decision.
Rendered for real into the plan's section 5 -- ready for an actual
decision.
src/ops_mason/{plan,executor,audit}.py: the phase-4 build executor for
credential_type openbao-approle-kv. Refuses to run against anything but
an approved plan -- verified the refusal never even calls subprocess.run.
role_id/secret_id (the AppRole's own access credential, not the
downstream secret) land as 0600 files, never logged; the HCL policy
goes over stdin, never argv; the audit trail is metadata-only. 12 tests,
all mocked at the bao boundary (no live OpenBao access from this
session).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
63 lines
3.2 KiB
Markdown
63 lines
3.2 KiB
Markdown
# Review / optimize checklist (phase 2)
|
|
|
|
Runs against a draft plan (`plans/<id>.md`, section 2-3 filled in) before
|
|
it's shown to anyone. Fill in section 4 ("Review notes") of the plan file
|
|
directly — this checklist is not a separate artifact, it's what section 4
|
|
*is*. A plan with an empty section 4 is not ready for phase 3.
|
|
|
|
This can be run by a human or an agent going through the plan by hand —
|
|
it does not need to be automated tooling before it's useful. Automating
|
|
individual checks (e.g. a script that diffs proposed TTLs against
|
|
existing lanes) is a reasonable future step once there's more than one
|
|
worked example to generalize a checker from — not before (same
|
|
generalize-from-a-second-example discipline the `glas-harness` reins use
|
|
for their own deferred decisions).
|
|
|
|
## The checks
|
|
|
|
1. **Naming convention.** Does every new object's name follow the
|
|
`<consumer>-<credential-purpose>` shape already visible in existing
|
|
lanes (`agent-harness-binky-mail`, `workload-kv-read-<path-slug>`)? A
|
|
plan proposing an unrelated naming scheme should be flagged, not
|
|
waved through for "this one's different."
|
|
|
|
2. **TTL/scoping match.** Do proposed `token_ttl`/`token_max_ttl`/
|
|
`token_num_uses`/`secret_id_ttl` values match an existing comparable
|
|
lane, or is there a stated reason to diverge? Silent divergence (a
|
|
shorter or longer TTL with no explanation) is a review finding, not a
|
|
detail to skip past.
|
|
|
|
3. **Redundancy check.** Does a policy, AppRole, or KV path already exist
|
|
that overlaps what's being proposed? This should already have been
|
|
asked in section 2 (existing-structure survey) — phase 2 is the
|
|
second pass confirming that answer still holds after the proposed
|
|
changes are fully drafted, not skipping it because section 2 already
|
|
looked.
|
|
|
|
4. **Compaction opportunity.** Does satisfying this demand make any
|
|
*existing* lane redundant or mergeable? Not every plan will find one —
|
|
most won't — but the check should be asked every time, not only when
|
|
it's obviously true.
|
|
|
|
5. **Ease of use for the consumer.** Is the credential-acquisition path
|
|
this plan builds toward straightforward for the actual consuming code
|
|
to use (e.g. does it match an env-var/file-path convention the
|
|
consumer's code already expects, per its own `credentials.py` or
|
|
equivalent)? A technically-correct plan that's awkward for the
|
|
consumer to actually integrate is a real review finding.
|
|
|
|
6. **Posture check.** Do the choices here (TTL length, rotation policy,
|
|
`secret_id_ttl` expiry-or-not) match current organizational posture
|
|
(`ops-warden/wiki/WorkloadSecurityPosture.md` — build phase today) —
|
|
not assuming production-tier rigor that isn't the current posture, and
|
|
not assuming dev-tier looseness forever. State the posture assumption
|
|
explicitly rather than leaving it implicit.
|
|
|
|
## Worked example
|
|
|
|
`plans/rein-openweights-openrouter-approle.md` section 4 applies all six
|
|
checks against a real plan — including one genuine finding (the TTL
|
|
values were carried over from `agent-harness-binky-mail` by direct
|
|
analogy rather than re-derived from scratch, which is exactly check 2
|
|
working as intended: matching an existing lane rather than inventing a
|
|
new number).
|