diff --git a/README.md b/README.md index d53d4cb..6c0bd7b 100644 --- a/README.md +++ b/README.md @@ -145,7 +145,7 @@ convention. See [`docs/CLONING.md`](docs/CLONING.md). The FAO delivery reads Appwrite connection settings from the environment. The required names are **declared** in `unfao/appwrite_env.py` and validated fail-loud before any store is constructed — a missing or empty variable raises, naming every one that is absent, -rather than half-configuring a client. Coordinates come from the PLATFORM-001 registry +rather than half-configuring a client. Coordinates come from the Appwrite Seam Contract registry (referenced by URL, never copied); the API key is an operator slot: ```bash @@ -157,9 +157,7 @@ APPWRITE_DATASTORE_API_KEY=... # Production-forecasts store (input) APPWRITE_PROD_FORECASTS_BUCKET_ID=production_forecasts APPWRITE_PROD_FORECASTS_BUCKET_NAME=Production Forecasts -APPWRITE_PROD_FORECASTS_COLLECTION_ID=... # TODO: verify against live Appwrite — the - # previously-documented `forecasts_metadata` - # was found NOT to exist (un_fao postmortem) +APPWRITE_PROD_FORECASTS_COLLECTION_ID=production_forecasts APPWRITE_PROD_FORECASTS_COLLECTION_NAME=Production Forecasts # UN FAO store (output) diff --git a/docs/ADRs/013_sampled_forecast_wire_contract.md b/docs/ADRs/013_sampled_forecast_wire_contract.md index 0b34d9b..7d2cf02 100644 --- a/docs/ADRs/013_sampled_forecast_wire_contract.md +++ b/docs/ADRs/013_sampled_forecast_wire_contract.md @@ -689,7 +689,7 @@ a "simplification." this contract governs the **data** crossing the Appwrite seam; the **identity/config** crossing the same seam (which key, which coordinates, sourced from where, named how) is governed by - **[PLATFORM-001 — Identity, Secrets & Configuration Contract (VIEWS Appwrite seam)](https://github.com/views-platform/views-appwrite/blob/main/docs/ADRs/platform/PLATFORM-001_identity_secrets_configuration_contract.md)**, + **[the Appwrite Seam Contract — Identity, Secrets & Configuration Contract (VIEWS Appwrite seam)](https://github.com/views-platform/views-appwrite/blob/main/docs/ADRs/platform/the Appwrite Seam Contract_identity_secrets_configuration_contract.md)**, homed in views-appwrite and referenced here **by URL, never by copy** — with its coordinate registry ([`coordinate_registry.toml`](https://github.com/views-platform/views-appwrite/blob/main/docs/ADRs/platform/coordinate_registry.toml)) @@ -1105,7 +1105,7 @@ record execution progress against it. assignment and the concrete period remain OPEN and are deliberately deferred — they must close before the first full-S production run, not before. - **2026-07-28 — þing-01 verdict executed in this repo (seam's other half - contracted).** The platform ratified **PLATFORM-001** (identity/secrets/ + contracted).** The platform ratified **the Appwrite Seam Contract** (identity/secrets/ configuration for the Appwrite seam, homed in views-appwrite) — the plumbing half of the seam whose data half this ADR seals. This repo's follow-through, all merged: **P1/#134** — the implicit ensemble-dotenv borrow @@ -1114,7 +1114,7 @@ record execution progress against it. every store construction (PR #137). **P2/#135** — delivery-log/provenance redaction audit: CLEAN; guards pin the wire/delivery packages credential-blind and the provenance keyset closed (PR #136). **P3/#138** — §7(d) added: this ADR - now cross-references PLATFORM-001 by URL, never by copy. + now cross-references the Appwrite Seam Contract by URL, never by copy. - **2026-07-31 — the Hop-A LEGACY READER IS RETIRED; §11.4's transition is over on this side (#149, epic #148).** §11.4 required a type-aware guard live at each diff --git a/docs/CLONING.md b/docs/CLONING.md index ae0c005..e149ce0 100644 --- a/docs/CLONING.md +++ b/docs/CLONING.md @@ -35,7 +35,7 @@ Four declarations, and nothing may be inferred: ### 2. Your store coordinates — `unfao/appwrite_env.py` The env-var names your delivery requires, validated fail-loud before any store is -constructed. Names come from the **PLATFORM-001 coordinate registry** (homed in +constructed. Names come from the **Appwrite Seam Contract's coordinate registry** (homed in views-appwrite) and are referenced **by URL at a pinned commit, never copied**. The secret stays an operator slot. @@ -53,7 +53,7 @@ defect became three (register C-40); pipeline-core declines to offer the surface that refusal is deliberate. Write a thin client against the SDK, as views-faoapi did. **Get your own key before the first run, not after.** -Free at t=0, a migration later. One key per identity per environment (PLATFORM-001 +Free at t=0, a migration later. One key per identity per environment (the Appwrite Seam Contract §5.3). Do not reuse another service's key — this repository ran for months under a key named for pipeline-core, and nobody could state its scopes from evidence. diff --git a/reports/technical_risk_register.md b/reports/technical_risk_register.md index eb16bec..3898349 100644 --- a/reports/technical_risk_register.md +++ b/reports/technical_risk_register.md @@ -5,8 +5,8 @@ | Project | views-postprocessing | | Owner | Dylan Pinheiro / PRIO MD&D Team | | Last Updated | 2026-08-01 | -| Total Concerns | 71 | -| Open Concerns | 24 | +| Total Concerns | 73 | +| Open Concerns | 26 | | Resolved Concerns | 47 | --- @@ -129,6 +129,10 @@ Tier recalibrated from 2 to 3 during review-rr (2026-06-02): the gap is maintain **Update 2026-06-24 (narrowed):** the `mapping.py` dimension is gone (C-39 — the `geopandas`/`shapely`/`joblib`/`multiprocessing` imports were deleted; `cachetools` dropped from `pyproject.toml`). Residual: `unfao.py` imports `pandas`/`polars`/`python-dotenv` undeclared, arriving transitively via `views-pipeline-core` (which *is* declared). Much smaller surface (Tier 4-ish); consider resolving outright if the transitive-via-pipeline-core guarantee is deemed sufficient. +**Update 2026-08-01 (`falsify`) — a SECOND undeclared dependency, and this one's transitive path is about to disappear.** `appwrite` is used in this repo (`contract/launch_config.py`, `unfao/managers/unfao.py`) and declared in **no** manifest — it arrives transitively via `views-pipeline-core`, exactly as `pandas` does. What makes it different from the pandas residual: **views-pipeline-core is making `appwrite` an optional extra** (their **#345**, on CRP grounds — three repos that never mention Appwrite currently install its SDK). **When that lands, the transitive path disappears and this repo breaks at import.** + +So this entry is no longer "Tier 4-ish, resolve if the transitive guarantee is deemed sufficient" — the guarantee is being **withdrawn upstream, deliberately**. Fix is one line: declare `appwrite` in `pyproject.toml`, or depend on `views-pipeline-core[appwrite]`. Relayed in **#172**; registered here rather than as a new entry because it is the same problem type at a new location. **New trigger: before views-pipeline-core#345 lands.** + **Update 2026-07-31 (review-rr — narrative corrected against the tree):** the 2026-06-24 residual is now overstated. Verified: **`polars` has zero references repo-wide**; **`python-dotenv` is dead** (þing-01 #134 killed the implicit ensemble-dotenv borrow — see `unfao/appwrite_env.py`); `cachetools` is gone. Meanwhile `views-frames` and `pyarrow` became **declared** direct dependencies. **The residual is `pandas` alone**, imported directly at the three locations above and arriving transitively via `views-pipeline-core`. One undeclared package on a path that is itself being retired (epic #85) — genuinely Tier 4-ish now; resolve outright if the transitive guarantee is deemed sufficient, or declare `pandas` explicitly in the same PR that closes #89. --- @@ -605,6 +609,10 @@ This repo declares exactly three dependencies — `views-pipeline-core`, `views- **⚠ Framing correction, recorded because it inverts the intuition that opened this investigation.** The audit began as "is geopandas back?" — and geopandas is the *architectural* violation (this repo spent PR #42 removing it). But it is **1.6 MB**. The *material* cost is **torch's NVIDIA CUDA stack at 2.5 GB — 89% of the entire virtualenv — in a delivery repo with no GPU code, no model training, and no tensor operations of any kind.** Optimising for the offensive dependency rather than the expensive one would have missed almost the whole bill. +**Update 2026-08-01 — the drag now has a measured security surface.** GitHub raised **32 Dependabot alerts** when `main` was brought current. **31 of them are `poetry.lock` resolution, not declared dependencies** — and the shape matches this entry exactly: **Pillow ×13** and **GitPython ×9** (via `plotly`/`seaborn`/`wandb`), plus `geopandas`, `torch`, `pytest`, `paramiko`, `pywin32`, `diskcache`, `setuptools`, `python-dotenv` — one each. Every one of those roots is declared by **views-pipeline-core 2.3.0**, none by this repo, and none is imported by this repo's code. + +This does not make the entry more urgent; it makes it **measurable**. The 3.0.0 bump this entry is held on removes the roots, and with them most of this alert surface. Until then the alerts are real but unreachable, and closing them individually would be treating symptoms of a dependency this repo does not choose. The 32nd alert is `pyarrow`, which **is** declared here and is registered separately as **C-72**. + **Dependency chain, traced 2026-07-31.** `views-pipeline-core` is the **sole** requirer: `torch = ">=2.6.0,<3.0.0"` in the 2.3.0 metadata; torch 2.12.1 then pulls the five `nvidia-*` wheels on Linux. Nothing else in the tree asks for torch, and this repo imports it zero times. **Removal path — there is no correct fix inside this repo.** views-pipeline-core `development` (already versioned **3.0.0**) declares **no torch, no geopandas, no scipy, no seaborn, no plotly**, and imports torch nowhere; their own test asserts the absence of `import torch`. The fix is complete upstream and purely a publishing gate. Their release runbook (#313) pins the order: **views-frames (done) → views-evaluation 0.5.0 → views-pipeline-core 3.0.0 → views-reporting 0.3.0 → models/postprocessing envs**, executing on the maintainer's platform-wide signal. So this entry closes when C-44's bump becomes takeable — not before, and not by local action. @@ -659,6 +667,58 @@ Cross-refs: **C-19** (RESOLVED — the ADR-008 log-before-raise sweep whose conv --- +### C-72: The pyarrow pin holds this repo inside a high-severity CVE, and the fix changes the wire bytes + +| Field | Value | +|-------|-------| +| ID | C-72 | +| Tier | 3 — **not currently exploitable here**: the CVE is a *read-path* use-after-free and this repo only **writes** arrow in production (the three `vf_arrow.load` calls are all in tests). It is Tier 3 rather than 4 because the bytes we write are read by views-faoapi under the same platform-wide pin, so the platform's exposure is real even though this repo's is not, and because the remedy collides with a ratified contract. | +| Source | `manual` (2026-08-01) — GitHub surfaced 32 Dependabot alerts on the push that made `main` current; reviewed alert by alert | +| Trigger | When lifting the `pyarrow < 17` ceiling — pipeline-core **#280** (the viewser → views-storage chain) is the platform half — **regenerate the ADR-013 §10 golden fixture in the same change and notify the faoapi seat**, because the upgrade is expected to change the delivered bytes. Also fires if this repo ever gains a production arrow **read**. | +| Location | `pyproject.toml:15` (`pyarrow = ">=16.1.0,<17.0.0"`); `poetry.lock`; the affected operation would be `views_frames.io.arrow.load`, called only at `tests/test_wire_shard.py:90`, `tests/test_wire_fixture.py:73`, `tests/test_wire_header.py:39` | + +**GHSA — "Apache Arrow: potential use-after-free when reading IPC file with pre-buffering."** Vulnerable range `>= 15.0.0, < 23.0.1`; patched in **23.0.1**. This repo's declared pin is `>=16.1.0,<17.0.0` — **entirely inside the vulnerable range, with a ceiling that excludes the fix.** It is the only one of the 32 alerts against a dependency this repo declares itself (`pyproject.toml`); the other 31 are `poetry.lock` resolution — see the disposition note below. + +**Why it is not exploitable here, stated precisely rather than reassuringly.** The defect is on the *read* path. Production writes arrow (`contract/wire/shard.py` → `views_frames.io.arrow`) and never reads it back; the only `vf_arrow.load` calls in the repository are the three byte-parity tests above, operating on committed fixtures we generated. So there is no path from partner or producer input to the vulnerable code **in this process**. + +**Where the exposure actually sits: views-faoapi.** It reads these shards, under the same platform-wide `pyarrow < 17` ceiling. Our output is its input. This entry does not claim to size that repo's risk — it records that the platform's pyarrow pin is a shared exposure and that the consumer is the reading half. + +**The collision worth understanding — the fix and the contract point in opposite directions.** The five byte-parity failures this repo has treated as "known local toolchain noise" all session are precisely this: the local environment runs **pyarrow 23.0.1 — the patched version** — while CI runs the pinned 16.1.x, and the two produce **different bytes** for the same input. So upgrading to the patched pyarrow is not a dependency bump; it **changes the §10 golden fixture**, which ADR-013 declares normative and which views-faoapi verifies against. Security remediation here is a **contract event**, not maintenance, and must be sequenced with the consumer. + +**⚠ TWO CORRECTIONS to this entry, made the same day it was written (2026-08-01), on evidence gathered while filing the cross-repo issues.** + +**(a) views-faoapi is NOT exposed, and the platform ceiling is not uniform.** This entry implied the consumer shared our pin. It does not: faoapi pins **`pyarrow==23.0.1`** — the patched version — in its `pyproject.toml:31`, and it is the repo that actually reads the wire (`forecast/ingestion/wire_reader.py` → `arrow.load`). So nobody on the platform is currently exposed through this path, and the `<17` ceiling views-pipeline-core#280 exists to lift is binding on the **producers** only. One consumer has already moved past it, in production. + +**(b) The re-vendor obligation is stronger than "our fixture changes".** ADR-013 §10 is explicit: *"All three implementing repos' test suites consume the same bytes … The other two repos **vendor a copy** and carry a **pinned root-hash equality test** … A change to the fixture is a change to the contract."* Verified — faoapi's copy is at `tests/forecast/golden/wire_contract/SHA256SUMS`, pinned by `tests/forecast/test_wire_golden_fixture.py`. So the upgrade is a **coordinated three-repo re-vendor**, and it carries an undecided question: whether it bumps `contract_version` (the payload schema does not change, only the encoder's bytes — §10 says a fixture change *is* a contract change). + +**Filed, so the relocation is complete rather than assumed** (the C-08 lesson): **views-postprocessing#174** (the coordinating issue), **views-faoapi#348** (heads-up + two questions only their seat can answer), and a comment on **views-pipeline-core#280** adding the security dimension and the non-uniform-ceiling finding. + +Cross-refs: **C-62** (the transitive dependency drag; the other 31 alerts), **C-46** (the datafactory version-state coupling), ADR-013 §10 (the byte-pinned fixture), views-pipeline-core **#280** (the platform pyarrow ceiling), views-faoapi **#348** (the reading half), **#174**. + +--- + +### C-73: The contract path selects the newest manifest over a broad filter — an unpaged upstream lookup can ship a stale run + +| Field | Value | +|-------|-------| +| ID | C-73 | +| Tier | 2 — a delivery can **assemble and ship a stale run rather than failing**. Not Tier 1 because the run that ships is internally coherent: its manifest hashes verify, its shards match, per-shard declared identity is checked. The partner receives a *complete, valid, wrong-vintage* delivery, and nothing on either side reports a problem. | +| Source | `falsify` (2026-08-01) — audit of "there is nothing more to do in this repo"; the hazard itself is views-pipeline-core's **C-241**, relayed to this seat in **#172** | +| Trigger | Before the next delivery run, and again when bumping to the pipeline-core release carrying their **#341** — confirm the run actually selected is the run expected (compare the resolved `run_id` against the producer's newest published run, not merely that a manifest resolved) | +| Location | `views_postprocessing/contract/wire/source_selection.py:40` (`HOP_A_MANIFEST_FILTERS`) and `:106` (`store.latest_file_id(dict(HOP_A_MANIFEST_FILTERS))`); reached through `views_postprocessing/unfao/managers/unfao.py:44` (`_ContractStorePort.latest_file_id`) → pipeline-core's `get_latest_file_id` → `search_files_by_metadata` | + +pipeline-core's `search_files_by_metadata` was **unpaged**: Appwrite returns 25 rows by default, and `get_latest_file_id` sorted *those* and called the result "latest". With more than 25 matching documents it returned **the newest of the oldest 25** — silently, and staler every run. + +**This reaches this repo because `HOP_A_MANIFEST_FILTERS` is broad by design**: `{"category": "forecast", "type": "sampled_forecast_manifest"}` matches **every manifest ever published**, and the store has no retention (C-25's own note: *"every historical upload remains a candidate forever"*). Shard lookups are name-scoped and unique (`:120`, `:144`), so they resolve correctly **for whichever run was selected** — which is precisely why the failure is quiet: everything downstream of the selection is self-consistent. + +**Not a defect in this repo's code, and the fix is not ours** — the upstream fix (pipeline-core #341) arrives with a pin bump because we import their `DatastoreModule` rather than keeping our own client. What *is* ours is that the register carried no record of it, and that the broad-filter-plus-newest-wins selection strategy is a choice this repo made and can revisit independently of the upstream fix. + +**⚠ This falsifies a claim made in C-25's resolution earlier the same day.** That entry was closed as *"superseded by mechanism"* on the reasoning that *"the contract path selects by run manifest … so recency-based selection is not an operation the code can perform any more."* **It is exactly what the code does** (`:106`). The half of C-25 that genuinely is solved is **producer identity** — the manifest carries a declared ensemble, verified per shard header (`:73-81`), so a *different producer's* run cannot be selected. The **recency** half was never solved; it moved from picking the newest payload file to picking the newest manifest. C-25's resolution has been corrected in place rather than rewritten, because the overclaim is more instructive than the conclusion. + +Cross-refs: **C-25** (whose resolution this corrects), **C-40** (the inherited pipeline-core surface this arrives through — **Cluster G**), **C-72** (the other pin-gated upstream item), **#172**, views-pipeline-core **#339** / **#341** / C-241, ADR-013 §4.3 (manifest selection). + +--- + ## Disagreements ### D-12: Post-Run-0 infrastructure & naming intents — repo rename, internal-store transport, compute co-location @@ -894,7 +954,7 @@ Cross-refs: **C-40** (which calls these the "retired-in-place legacy seams"), ** |-------|-------| | ID | C-25 | | Resolved | 2026-07-31 | -| Resolution | **Superseded by mechanism (#150, epic #148) — not merely mitigated.** This entry's hazard was "newest `category=forecast` upload wins regardless of producer". Its recorded mitigation was `delivery.identity.assert_forecast_identity`, which #150 retired. The hazard nonetheless cannot occur: since #149 the contract path is the only path, and it selects by **run manifest** — a commit marker whose contents are hash-verified — so recency-based selection is not an operation the code can perform. Declared identity is additionally checked per shard header at `wire/source_selection.py:73-81`. Closed as structurally impossible rather than guarded. | +| Resolution | **Superseded by mechanism (#150, epic #148) — not merely mitigated.** This entry's hazard was "newest `category=forecast` upload wins regardless of producer". Its recorded mitigation was `delivery.identity.assert_forecast_identity`, which #150 retired. The hazard nonetheless cannot occur: since #149 the contract path is the only path, and it selects by **run manifest** — a commit marker whose contents are hash-verified — so recency-based selection is not an operation the code can perform. Declared identity is additionally checked per shard header at `wire/source_selection.py:73-81`. Closed as structurally impossible rather than guarded.

**⚠ CORRECTION 2026-08-01 — this resolution overclaimed, and the overclaim is recorded rather than rewritten.** *"Recency-based selection is not an operation the code can perform any more"* is **false**: `contract/wire/source_selection.py:106` calls `store.latest_file_id(dict(HOP_A_MANIFEST_FILTERS))` — it selects the **newest** manifest over a filter matching every manifest ever published. What manifest selection actually solved is the **producer-identity** half of this entry's hazard: the manifest declares its ensemble and every shard header is verified against it (`:73-81`), so another producer's run cannot be selected. The **recency** half was not solved — it moved from newest-payload-file to newest-manifest. That half is now **C-73**, which is live and Tier 2. This entry stays resolved for the identity hazard it was written about; do not read it as covering run vintage. | | Tier | 2 | | Source | `cross-repo-investigation` (2026-06-12) | | Trigger | When any new producer uploads to the prod_forecasts bucket with `category: "forecast"`, verify the postprocessor still picks up the intended ensemble's file — selection is newest-`$createdAt`-wins with no loa, model-name, or run-id filter |