# v0.9.9 — Approval Authority — adversarial review

**Date:** 2026-04-30 (post-publish)
**Reviewer:** Codex CLI 0.118.0, in adversarial / "challenge" mode
**Scope:** v0.9.8..v0.9.9 trust-surface diff (PRs #50–#55)
**Verdict:** **NOT SHIP-AS-IS.** v0.9.10 patch warranted. Four real blockers found and verified against the code.

---

## Threat model used

The 10 invariants the verifier was challenged to break:

1. Nonces bind: action's `meta.approval_use_id` references a `ApprovalUse` whose `nonce_digest` matches the grant's signed nonce.
2. Scopes authorize: actor / action / subject match approval scope.
3. Approval Uses bind to action artifact, not free-floating.
4. Local journal enforces local replay: `(grant_id, nonce_digest)` exceeding `max_uses` is rejected.
5. Package export makes approval evidence portable and round-trippable.
6. Included-checkpoint verification offline + tamper-evident via `record_digest` recompute.
7. Hub/org replay PASS only when (a) checkpoint declares `kind: hub-org`, (b) every required Hub field present, (c) Ed25519 signature verifies, (d) every `ApprovalUse.use_id` ∈ `covered_use_ids`.
8. No raw commands, prompts, file contents, bearer tokens, or secrets in approval-use evidence.
9. Strict mode (`--strict`) fails on tamper / scope / coverage violations.
10. Default mode warns without overclaiming. Never says "global single-use enforced" without verified evidence.

---

## BLOCKERS (verified against code)

### Blocker 1 — TOCTOU race in `reserve_in_journal` bypasses `max_uses`

- **Files:** `packages/cli/src/commands/attest.rs:660–746`, `packages/core/src/journal/mod.rs:221–280` (`with_lock` + `append_use`)
- **Invariant violated:** 4 (local journal enforces local replay).
- **Repro:**
  ```
  treeship attest action --approval-nonce N --max-uses 1 --grant <gid> <action>
  ```
  Run twice in parallel against a fresh journal. Both processes pass `journal::check_replay` BEFORE the append lock is acquired. Each then calls `journal::append_use`, which serializes through `with_lock`. The lock prevents simultaneous filesystem writes, but neither writer re-checks replay after acquiring the lock — so both records get written. `max_uses=1` is exceeded.
- **Where the gap is:** `reserve_in_journal` calls `check_replay` (line ~679) and `list_uses_for_grant` (line ~692) entirely outside `with_lock`. `append_use` takes the lock internally only for write, not for the replay decision.
- **Fix sketch:** Move `check_replay` and the `prior_count` derivation INSIDE `with_lock` — re-check that `existing_count + 1 ≤ max_uses` after acquiring the lock. Equivalent to the SQL `SELECT … FOR UPDATE` pattern. Tests must include a parallel-attest case.

### Blocker 2 — `meta.approval_use_id` ignored at verify time

- **Files:** `packages/cli/src/commands/session.rs:1397–1495` (`collect_approval_evidence`), `packages/core/src/journal/mod.rs:680–683` (the `find_use_for_action` comment that flags this gap).
- **Invariant violated:** 3 (Approval Uses bind to action artifact).
- **Repro:**
  1. Run a real session with one approved action; close session; export package.
  2. Open the action artifact's JSON inside the package; mutate `meta.approval_use_id` to a nonexistent or unrelated `use_id` (or delete the field).
  3. Run `treeship package verify <pkg>`. Verify still passes — including under `--strict`.
- **Where the gap is:** `collect_approval_evidence` resolves uses purely by `(grant_id, nonce_digest)`, then calls `journal::list_uses_for_grant(&grant_id)` and dumps every matching use into the bundle. The action artifact's `meta.approval_use_id` is mentioned in a comment (line 1462–1463) but never used to filter or validate. The explicit pointer is decorative.
- **Fix sketch:** When an action carries `meta.approval_use_id`, require that the resolved use_id matches. Otherwise emit a `approval-use-binding` failure row.

### Blocker 3 — `nonce_digest` in `ApprovalUse` not cross-checked against grant's signed nonce

- **Files:** `packages/core/src/session/package.rs:524–584` (`add_approval_evidence_checks` `replay-package-local` branch), `packages/core/src/session/package.rs:614–632` (`approval-use-integrity`).
- **Invariants violated:** 1 (nonces bind), 5 (package evidence portable).
- **Repro:**
  1. Export a package containing one `ApprovalUse` and its grant.
  2. Edit `approvals/uses/<use_id>.json`: change `nonce_digest` to a different value. Recompute `record_digest` from the canonical form (it includes `nonce_digest`, so individual digest stays consistent).
  3. Run `treeship package verify --strict <pkg>`. Passes.
- **Where the gap is:** `add_approval_evidence_checks` reads `u.nonce_digest` directly from the use record. The grant's signed nonce (in the bundle's grants list) is never hashed and compared. The `approval-use-integrity` check only re-derives `record_digest` from the use's own fields — so an internally consistent forgery passes.
- **Fix sketch:** In `add_approval_evidence_checks`, pull the grant for each use, compute `nonce_digest(grant.nonce)`, compare to `use.nonce_digest`. Mismatch → fail (default warn, strict fail).

### Blocker 4 — Journal chain integrity not verified in package replay

- **Files:** `packages/core/src/journal/mod.rs:484–561` (`verify_integrity` exists but unused in package path), `packages/core/src/session/package.rs:524–632` (`add_approval_evidence_checks` does NOT call any chain walk).
- **Invariant violated:** 6 (included-checkpoint tamper-evident).
- **Repro:**
  1. Export a package with multiple uses + a `LocalJournal` checkpoint.
  2. In `approvals/uses/`, rewrite the chain consistently: insert a forged use record, update the next record's `previous_record_digest` to point at the forgery, recompute each record_digest end-to-end. Update the checkpoint's `previous_record_digest` to anchor on the rewritten tail. All individual digests recompute correctly.
  3. Run `treeship package verify --strict <pkg>`. Passes.
- **Where the gap is:** `replay-included-checkpoint` only recomputes each checkpoint's *individual* `record_digest`. `approval-use-integrity` only recomputes each use's *individual* `record_digest`. Neither walks the `previous_record_digest` chain across embedded records. `verify_integrity` is a complete chain walker (484–561) but lives on the workspace journal and is never invoked on the package's embedded sub-chain.
- **Fix sketch:** Add a `replay-package-chain` row that orders the embedded uses + checkpoints by `previous_record_digest`, recomputes each digest, and validates the chain links match. The chain anchor for offline replay is either: (a) a Hub-signed checkpoint at the tail, or (b) explicit "anchor unknown" warning. Without an anchor, the chain is *internally consistent* but not provably non-rewritten — make that explicit in the output rather than silent.

---

## IMPORTANT NON-BLOCKERS

- Hub-org replay (`replay-hub-org`) signature + coverage logic is correctly wired. `canonical_hub_signing_bytes` includes every signing-relevant field, signature is verified against embedded public key, coverage is per-use. The four-gate rule from PR #55 holds. **Files:** `packages/core/src/statements/approval_use.rs:523–581`, `packages/core/src/session/package.rs:632–730`.
- Strict-mode promotion logic is correctly implemented in the CLI verify path. The gap is in what the underlying checks actually prove (Blockers 1–4), not in whether `--strict` escalates them. **Files:** `packages/cli/src/commands/package.rs:1074–1091`.
- Hub public-key substitution (attacker controls package, declares own `hub_public_key`, signs with matching private key) is a known v0.9.9 trust-model limitation, not a new bypass. The CHANGELOG and docs already call this out as deferred to a future "issuer / registry identity" release. **Files:** `packages/core/src/statements/approval_use.rs:550–568`.

---

## COPY / CLAIM ISSUES

- **`approval-use-integrity` check label** — under-claims integrity. The label implies the use record's *content* is integrity-checked, but the check only re-derives `record_digest` from the use's own fields and does not validate `nonce_digest` against the grant's signed nonce. Either rename to `approval-use-record-digest` or strengthen the check.
- **Default-mode `replay-*` PASS rendering** — when underlying evidence binding is incomplete (Blockers 2–4), a `✓` row currently prints "passed" even though the proof is partial. Default mode should either downgrade to `consistent / unverified-pass` wording or condition the `✓` on the full proof chain. Today's rendering risks the exact "global single-use enforced" overclaim the v0.9.9 honesty rule was supposed to prevent.

---

## TESTS TO ADD (post-fix)

| Test | Invariant | Suggested location |
|---|---|---|
| `attest_max_uses_race_fails_when_parallel` | 4 (concurrent replay) | `packages/cli/tests/attest.rs` |
| `action_meta_use_id_missing_is_rejected` | 3 (action↔use binding) | `packages/cli/tests/session.rs` or new integration |
| `package_verify_rejects_use_nonce_mismatch` | 1, 5 (nonce binding) | `packages/core/tests/session_package.rs` (new) |
| `package_verify_rejects_forged_journal_chain` | 6 (chain tamper-evidence) | `packages/core/tests/session_package.rs` |
| `strict_mode_promotes_replay_and_integrity_warnings_to_fail` | 9 (strict honesty) | `packages/cli/tests/package.rs` |

---

## Triage decision

| Finding | Severity | Action |
|---|---|---|
| Blocker 1 — TOCTOU race | High (replay bypass) | **v0.9.10 patch** |
| Blocker 2 — action↔use unenforced | High (binding bypass) | **v0.9.10 patch** |
| Blocker 3 — nonce_digest unbound | High (binding bypass) | **v0.9.10 patch** |
| Blocker 4 — chain unverified in package | High (tamper-evidence bypass) | **v0.9.10 patch** |
| Copy: `approval-use-integrity` underclaim | Medium | Same patch (label + behavior) |
| Copy: default-mode `✓` overclaim | Medium | Same patch (rendering rule) |
| Hub key substitution (no issuer registry) | Known limitation | Already documented as deferred; out of scope here |
| Tests post-fix | Required | Land alongside each fix |

Per release policy (replay bypass → cut patch immediately), all four blockers warrant **v0.9.10**. v0.9.9 packages should remain readable but verifiers calling `--strict` against tampered packages cannot assume safety until v0.9.10 lands.

---

## Codex pass attestation

- Tool: `codex-cli 0.118.0` at `/Users/zzo/.nvm/versions/node/v22.20.0/bin/codex`
- Mode: targeted adversarial review with the threat model above pasted in
- Subagent: `codex:codex-rescue` — synthesized findings; raw transcript not preserved
- Verification: each Codex-cited file:line was independently read and the gap reproduced in the actual code before being recorded as a confirmed blocker. No Codex finding was accepted on its own; this document records only verified gaps.

---

## v0.9.10 PR A — round 2 re-check

After the four blockers above were closed in `fix/0.9.10-approval-authority-patch` (commit `e62127a`), a second targeted Codex pass scoped to the fix diff confirmed Blocker 1 was closed but flagged three adjacent gaps the first-round fixes had introduced.

### Round-2 findings

**RE-CHECK VERDICT (round 2):**
- Blocker 1 (TOCTOU race): closed
- Blocker 2 (action↔use binding): partially closed
- Blocker 3 (nonce binding): partially closed
- Blocker 4 (chain continuity): partially closed

**Round-2 confirmed sub-blockers**

1. **Action envelope content not verified.** `read_approvals_bundle` loads `artifacts/*.json` and treats the filename stem as the artifact_id. `approval-use-action-binding` parses the envelope and reads `meta.approval_use_id` but never re-derives the artifact_id from the envelope's PAE bytes. An attacker controlling the package can write any forged unsigned action JSON to `artifacts/<id>.json` and the binding row trusts it. — `packages/core/src/session/package.rs` L373, L385, L780.
2. **Grant envelope content not verified.** `approval-use-nonce-binding` hashes `grant.nonce` from the parsed grant envelope, but the envelope is only parsed, not content-id-verified, before its nonce is trusted. A forged grant payload with any nonce makes the row pass. — `packages/core/src/session/package.rs` L707, L712.
3. **Chain continuity only checks set membership.** A fully rewritten self-consistent chain, mid-chain `previous_record_digest == ""`, disconnected embedded subchains, and cycles all still pass. — `packages/core/src/session/package.rs` L868.

**Round-2 secondary items**
- No explicit retry-after-`LockBusy` test for callers reusing the same `idempotency_key`.
- New package tests use dummy envelope signatures; no signature-validation path exercised.

### Round-2 resolution (commit follows)

1. **Content-addressing gate on action envelopes.** In `add_approval_evidence_checks`, before parsing each `bundle.action_envelopes[i].1` for `meta.approval_use_id`, re-derive `artifact_id_from_pae(pae(payload_type, payload))` and require it to equal the filename stem (`bundle.action_envelopes[i].0`). Mismatch → `approval-use-action-binding` FAIL with detail "envelope substituted or tampered." Real envelopes pass because they were signed over the same PAE bytes that produced the artifact_id; only forgeries fail.
2. **Content-addressing gate on grant envelopes.** Same shape: derive the grant_id from the envelope's PAE bytes, require it to equal the bundle's `grants[i].0`. Failure → `approval-use-nonce-binding` FAIL.
3. **Tightened chain continuity.** Algorithm rewritten to require:
   - exactly one record with `previous_record_digest == ""` (genesis);
   - no two records sharing the same non-empty `previous_record_digest` (forks);
   - no cycles (visited set during walk);
   - every record reachable from the genesis (no disconnected subchains).
   Each violation surfaces with a specific detail message.
4. **Idempotency-retry regression test.** Pinned in `journal::tests::reserve_use_retry_after_lock_busy_does_not_bypass_max_uses`: five sequential retries against the same `(grant_id, nonce_digest)` after an initial successful reserve all return `MaxUsesExceeded`; on-disk record count stays at 1.

### Round-2 tests added

| Test | Pins | File |
|---|---|---|
| `forged_action_envelope_under_wrong_id_fails_content_addressing` | Action envelope content addressing | `packages/core/tests/approval_evidence_binding.rs` |
| `forged_grant_envelope_under_real_id_fails_content_addressing` | Grant envelope content addressing | `packages/core/tests/approval_evidence_binding.rs` |
| `multiple_genesis_records_fail_chain_continuity` | Single-genesis invariant | same |
| `forked_chain_two_records_share_prev_fails_chain_continuity` | No-fork invariant | same |
| `disconnected_subchain_fails_chain_continuity` | Connected-from-genesis invariant | same |
| `reserve_use_retry_after_lock_busy_does_not_bypass_max_uses` | Idempotency retry safety | `packages/core/src/journal/mod.rs` |

After round-2 patch: 251 core unit tests + 17 binding integration tests + 7 hub-checkpoint integration tests + acceptance smoke all green.

### Round-3 Codex re-check

Scope: did round 2 close the three sub-blockers, and did it introduce any new bypass adjacent to the same surface?

**Round-3 verdict (per Codex):**
- Round-2 fix #1 (action content-addressing): **closed**
- Round-2 fix #2 (grant content-addressing): **closed**
- Round-2 fix #3 (chain continuity tighten): **closed**
- New blockers introduced by round 2: **(none)**

**Overall:** ship. PR A is merge-ready once CI lands.

**Secondary follow-ups (non-blocking, tracked for next hardening pass — NOT merge gates):**

1. Duplicate `grant_id` entries in `bundle.grants` are not explicitly rejected; the `grant_nonce_digest` HashMap silently last-wins. Real packages produced by `build_package_with_approvals` deduplicate by sanitized filename, so this is hard to trigger in production, but a defensive explicit-failure check (`return Fail` on second insertion under the same id) would make the row's behavior loud and unambiguous. — `packages/core/src/session/package.rs:709–744`.
2. Test coverage for chain shapes that pass the algorithm by accident is partial. Round-2 tests cover dangling, multiple genesis, fork (two records sharing prev), and disconnected/2-cycle. Missing: a 3-cycle (a→b→c→a) and a self-loop (record's own digest equals its previous_record_digest). The current algorithm should reject both via the visited-set walk, but a regression test should pin that. — `packages/core/tests/approval_evidence_binding.rs`.

Both are filed in the v0.9.10 follow-ups list and will land in the next approval-authority hardening PR (not v0.9.10 itself, since neither is a bypass).

### Final triage decision

| Round | Findings | Action |
|---|---|---|
| Round 1 (v0.9.9 publish) | 4 verified blockers | v0.9.10 PR A round-1 commit `e62127a` |
| Round 2 (re-check on round 1) | 3 adjacent sub-blockers | v0.9.10 PR A round-2 commit `29a46a3` |
| Round 3 (re-check on round 2) | 0 blockers, 2 non-blocking secondaries | Ship; secondaries deferred to follow-up |

PR A status after round 3: **merge-ready** pending the last cross-SDK macOS matrix job and the Codex secondaries get filed as a follow-up issue.
