# M20-I2-CHARTER-FILBERT-1 (#1500) — charter and prerequisite verdict

Reviewer: Filbert. Role disclosure: I was the increment 1 independent reviewer; that approval covered the fifteen tests' assertions, not comprehensive registry safety, and this review relies on fresh reproduction only. Baseline `b3fa2210…`; frozen draft `/tmp/m20-i2-charter-review-fga9lki3` at manifest SHA-256 `78981e71521e5df2b62649fe11eda6c241029c4fe41ef5dfe06fff3776f44401` (both hashes verified). All reproduction used a disposable `git archive` tree and synthetic fixtures; no real credentials, Pi, timers, Docker, packages, source edits, or shared logs.

## Verdict: APPROVED (charter), with prerequisite correction confirmed as mandatory stage 1

The charter is well-formed and its dependency ordering is right: nothing may build on the validator until the reproduced defects are closed and independently verified. The factual findings below are separately recorded from the proposed design, per the request.

## Factual prerequisite findings (all reproduced on the clean baseline)

- **D1 — fail-open root (the coordinator's defect, independently reproduced):** a nonexistent registry root and an empty root both return `valid`, exit 0; `list` exits 0 with an empty projection. `safeList` converts every directory error into an absent directory.
- **D2 — symlink/containment failure:** a provider file symlinked from outside the root with a matching id returns `valid` exit 0; an account directory symlinked from outside the root is followed, its records validated and listed. The loader uses `stat`/`readFile`, both of which follow symlinks; there is no containment check anywhere.
- **D3 — version schema inconsistency:** numeric `providerVersion: 2` is rejected (`invalid-id`) while the governing plan's examples use numeric `1`; arbitrary strings such as `"banana-schema"` are accepted. Wrong in both directions.
- **D4 — nested unknown keys pass:** unknown fields inside `harnesses.pi` (e.g. injected `secretExfil`) validate clean; harness entries are only checked to be objects.
- **D5 — enrollment not enforced:** a profile default account absent from `allowedAccounts` validates clean.
- **D6 — credential-type match not enforced:** an `oauth` account under a provider whose `credentialTypes` is `["api_key"]` validates clean.
- **D7 — non-pi endpoints unvalidated:** a `custom-endpoint` provider carrying a plain-`http` baseUrl on a non-pi harness produces no insecure-transport error (I1-F3, now confirmed as a prerequisite defect rather than later hardening).
- **D8 — URL credentials accepted:** `https://user:secret@host/v1` as a pi baseUrl returns `valid`.
- **D9 — bounded content echo in errors:** malformed JSON surfaces V8's parse message including a truncated excerpt of the file's first bytes (`"FIXTURE-MA"...`). Bounded to a short prefix, but a credential-shaped file misnamed `.json` would leak its opening characters.
- **D10 — partial trusted output:** `list` prints its projection even when validation errors exist (exit 1, output still emitted). Also observed in passing: a missing `account.json` inside a listed account directory reports as `invalid-json: ENOENT`, a code mislabel.

D1–D10 confirm every concern in the charter's prerequisite gate. None were covered by the increment 1 suite's assertions; two (D7, partially D4) were flagged in my I1 notes as hardening and now stand as blocking prerequisites.

## Charter assessment

- **Dependency ordering:** correct. Stage 1 (close verified defects, independent approval, clean-tree regression) precedes resolution, materialization, and the refresh adapter. Given D1/D2, building a materializer on this validator would generate inputs from absent or out-of-root sources while reporting success.
- **Scope:** confined to `packages/mosaic` plus assignment records; `scripts/auth.sh`, launchers, native homes, dataRoot, services, images, releases untouched; no new dependencies without reviewed need. The stage-2 boundary against implementing #53's full runtime model is stated.
- **No-secret guarantees:** fake-marker disclosure tests for stdout/stderr/errors, no credential hashes in manifests, token-bearing output never logged, fake-Pi refresh simulation with isolated auth location, `--credentials`/print-* exclusion. Adequate, and now demonstrably necessary (D9).
- **Rollback:** source-level revert; tests clean synthetic temp directories; no runtime migration performed. Consistent.
- The closing self-correction (prior `valid` responses were labeled acceptance too strongly) is accurate and appropriately does not waive anything.

## Recommended corrective scope (the separate narrow prerequisite increment)

1. Fail-closed roots: refuse missing, inaccessible, or non-directory registry roots and required subdirectories before any record processing.
2. Containment: `lstat`-based regular-file checks and no symlink following for files or directories anywhere under the supplied root.
3. Version fields: numeric versions per the governing plan, with unknown-version refusal; migrate fixtures accordingly in the same patch.
4. Nested shape validation for every harness entry (unknown keys, per-kind requireds), applied to all harnesses, not only `pi`; reject URLs carrying `user:pass@` and confine `http` to `allowInsecureTransport`.
5. Cross-record enforcement: default accounts enrolled in `allowedAccounts`; account `type` within the provider's `credentialTypes`.
6. Bounded secret-free errors: fixed invalid-JSON code without parse-message excerpts; correct the ENOENT mislabel; `list` must not emit trusted projections on an invalid tree.
7. Negative controls in the package suite for each item above, including fake-marker disclosure assertions.

This correction is small, independently reviewable, and should land before any stage-2 work. The charter already mandates exactly that ordering; approving the charter does not approve any implementation.