Skip to content

S10 (#192) — repair the þing-01 redaction guard; a declared root that vanishes now fails #204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Polichinel merged 1 commit into from
Aug 2, 2026
Merged

S10 (#192) — repair the þing-01 redaction guard; a declared root that vanishes now fails #204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 46 additions & 36 deletions reports/technical_risk_register.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
| Owner | Dylan Pinheiro / PRIO MD&D Team |
| Last Updated | 2026-08-02 |
| Total Concerns | 74 |
| Open Concerns | 18 |
| Resolved Concerns | 56 |
| Open Concerns | 17 |
| Resolved Concerns | 57 |

---

Expand Down Expand Up @@ -47,7 +47,7 @@ covered a single open entry (see Historical clusters below).
**Root cause:** the register, ADR prose, and issue bodies are hand-maintained mirrors of cross-repo state that moves under them.
**Entries:** C-44, C-46, C-47, C-57, C-74 (a guard whose declared scan roots silently stopped existing — the cluster's disease inside the cluster's own prescription) — plus this register's own findings at review-rr 2026-07-31 (header miscount, two RESOLVED entries misfiled under Open, eight stale `unfao.py` line ranges after the manager grew 273→636 lines, two unnamespaced foreign-register IDs). Historical precedent: the entire C-48–C-55 ADR-013 audit series, and C-42/C-47.
**Highest tier:** 3
**Fix strategy:** this repo already solved this disease once — the ADR-013 audit series ended with **40 permanent guard tests** (`tests/test_falsify_adr013_*.py`), and the same pattern now guards the þing-01 invariants (`tests/test_env_declaration.py`, `tests/test_redaction_guard.py` — the latter **only over the roots that still exist**, see C-74: a guard is only as good as the assertion that its inputs are real). There is **no equivalent for the register**. A small `tests/test_register_integrity.py` — header counts match section counts; no RESOLVED body under `## Open Concerns`; every `C-\d+`/`D-\d+` reference resolves or is namespaced to a foreign register — would make this class self-detecting.
**Fix strategy:** this repo already solved this disease once — the ADR-013 audit series ended with **40 permanent guard tests** (`tests/test_falsify_adr013_*.py`), and the same pattern now guards the þing-01 invariants (`tests/test_env_declaration.py`, `tests/test_redaction_guard.py` — the latter briefly **only over the roots that still existed**, see C-74, resolved: a guard is only as good as the assertion that its inputs are real, and it now carries that assertion). There is **no equivalent for the register**. A small `tests/test_register_integrity.py` — header counts match section counts; no RESOLVED body under `## Open Concerns`; every `C-\d+`/`D-\d+` reference resolves or is namespaced to a foreign register — would make this class self-detecting.
**Resolution scope:** Full for the mechanical half.

### Cluster J: Delivery aftercare has no mechanism
Expand Down Expand Up @@ -536,38 +536,6 @@ Cross-refs: **C-25** (whose resolution this corrects), **C-40** (the inherited p

---

### C-74: The þing-01 redaction guard scans four paths that stopped existing — it reports success while covering one root of five

| Field | Value |
|-------|-------|
| ID | C-74 |
| Tier | 3 — **not a leak today**: the relocated modules were checked directly and are still credential-blind (zero hits for `os.environ`, `getenv`, `load_dotenv`, `API_KEY`, `credentials`). What is gone is the thing that would notice them ceasing to be. A security-adjacent control that cannot fail is a maintainability defect until the day it is a correctness one. |
| Source | `review-diff` (2026-08-02) — S1/#182 review; found while reading the redaction discipline the new ADR-008 test cites as "the wider rule" |
| Trigger | When any module under `contract/wire`, `contract/historical.py`, `contract/track_a_source.py` or `contract/frame_extraction.py` gains environment access — the #135 guard will not report it. Also fires on **the next package move**: `rglob` on a vanished root yields silence, not an error, so any future relocation narrows the scan again with no signal |
| Location | `tests/test_redaction_guard.py:25-31` (`_CREDENTIAL_BLIND`), `:34-39` (`_python_sources`) |

The þing-01 delivery-log redaction audit (#135, orð_09 §3) certified five module trees as credential-blind and pinned that finding as a permanent guard. Epic #148's S5 (#153) then moved the machinery out from under `unfao/` into `contract/`. **Four of the five roots were never re-pointed:**

| declared root | exists | files scanned |
|---|---|---|
| `unfao/wire` | no | 0 |
| `delivery` | yes | 6 |
| `unfao/historical.py` | no | 0 |
| `unfao/track_a_source.py` | no | 0 |
| `unfao/frame_extraction.py` | no | 0 |

**The failure is silent by construction.** `_python_sources` branches `if path.is_file(): … else: path.rglob("*.py")`, and `rglob` on a **nonexistent** directory yields an empty iterator rather than raising. A missing root and a clean root are therefore indistinguishable to the test, which passes either way. The guard reports an audited fact as pinned while pinning roughly a third of it.

**Why this is Cluster I and not merely a stale path.** The register's own text asserts the opposite. **C-57** says, in its "deliberately out of scope" paragraph, that *"the þing-01 redaction clause is **already mechanically enforced** (`tests/test_redaction_guard.py` — the delivery modules stay credential-blind…)"* — and **Cluster I's fix strategy cites this very file** as an example of the guard pattern working. Both were written in good faith and both are now overclaims. That is the cluster's disease reproducing inside the cluster's own prescription.

It is also the **sixth** instance of the stale-claim class found in two days, after C-63 and C-47 (filed Open with the defect fixed), C-43/C-59/C-61 (filed Open with their stated closing conditions met), #158 (closed with the code half undone) and `tests/test_validation.py` (claiming fidelity to a method it no longer resembles — **C-03**). The common shape is not carelessness: it is that a *move* leaves prose and paths behind, and nothing in this repo asserts that a declared path exists.

**Mitigation:** re-point the four roots at `contract/`, and add a root-existence assertion so a future relocation fails loudly instead of silently narrowing. The second half is the load-bearing one — re-pointing fixes today, asserting existence fixes the class. Tracked as a story under **epic #181**.

Cross-refs: **C-57** (whose "already mechanically enforced" claim this falsifies — corrected in place), **C-03** (the same class in `test_validation.py`), **C-46** (a different guard that also does not run, by a different mechanism), **C-63**, **C-47**, **Cluster I**, #135, #153, #182.

---

## Disagreements

### D-12: Post-Run-0 infrastructure & naming intents — repo rename, internal-store transport, compute co-location
Expand Down Expand Up @@ -636,6 +604,48 @@ See also C-40 (the inheritance/representation coupling this migration unwinds),

## Resolved Concerns

### C-74: The þing-01 redaction guard scans four paths that stopped existing — it reports success while covering one root of five — RESOLVED

| Field | Value |
|-------|-------|
| ID | C-74 |
| Resolved | 2026-08-02 |
| Resolution | **Closed by S10 (#192).** The four roots that #153 orphaned now point at `contract/`, and the scan covers **17 files where it covered 6**. Verified 2026-08-02.

**The half that matters is the second one.** Re-pointing fixes today; `test_every_declared_credential_blind_root_exists` fixes the class. `rglob` on a nonexistent directory yields an empty iterator rather than raising, so a missing root and a clean root were indistinguishable — a package move silently emptied the scan while the suite stayed green. A declared path that does not exist is now an error, not silence. A third check bounds the count, so narrowing shows up as a number rather than as nothing.

Mutation-proven by restoring the pre-S10 roots: **two failures**, one naming all four missing paths, one reporting `covers only 6 files`. The guard was also proven to bite on the defect it was written for — a synthetic module reading `os.environ['APPWRITE_DATASTORE_API_KEY']` — which had never been demonstrated, and an unproven guard is what two silent days buys you.

**List, not derivation — decided and recorded.** `tests/test_clone_readiness.py` enumerates an overlapping module set for a *different* question ("does the machinery import without the partner?"). Folding them together would couple two guards whose sets are free to diverge: a module can be partner-neutral without being credential-blind. Two lists that each say what they mean beat one that means neither. What makes the explicit list safe is the existence assertion — without it, a list is exactly the fragile thing it looked like here.

**No leak occurred.** The relocated modules were checked directly at registration and again here: zero hits for `os.environ`, `getenv`, `load_dotenv`, `API_KEY`, `credentials`. The audited fact stayed true; what had gone was the thing that would notice it stopping. |
| Tier | 3 — **not a leak today**: the relocated modules were checked directly and are still credential-blind (zero hits for `os.environ`, `getenv`, `load_dotenv`, `API_KEY`, `credentials`). What is gone is the thing that would notice them ceasing to be. A security-adjacent control that cannot fail is a maintainability defect until the day it is a correctness one. |
| Source | `review-diff` (2026-08-02) — S1/#182 review; found while reading the redaction discipline the new ADR-008 test cites as "the wider rule" |
| Trigger | When any module under `contract/wire`, `contract/historical.py`, `contract/track_a_source.py` or `contract/frame_extraction.py` gains environment access — the #135 guard will not report it. Also fires on **the next package move**: `rglob` on a vanished root yields silence, not an error, so any future relocation narrows the scan again with no signal |
| Location | `tests/test_redaction_guard.py:25-31` (`_CREDENTIAL_BLIND`), `:34-39` (`_python_sources`) |

The þing-01 delivery-log redaction audit (#135, orð_09 §3) certified five module trees as credential-blind and pinned that finding as a permanent guard. Epic #148's S5 (#153) then moved the machinery out from under `unfao/` into `contract/`. **Four of the five roots were never re-pointed:**

| declared root | exists | files scanned |
|---|---|---|
| `unfao/wire` | no | 0 |
| `delivery` | yes | 6 |
| `unfao/historical.py` | no | 0 |
| `unfao/track_a_source.py` | no | 0 |
| `unfao/frame_extraction.py` | no | 0 |

**The failure is silent by construction.** `_python_sources` branches `if path.is_file(): … else: path.rglob("*.py")`, and `rglob` on a **nonexistent** directory yields an empty iterator rather than raising. A missing root and a clean root are therefore indistinguishable to the test, which passes either way. The guard reports an audited fact as pinned while pinning roughly a third of it.

**Why this is Cluster I and not merely a stale path.** The register's own text asserts the opposite. **C-57** says, in its "deliberately out of scope" paragraph, that *"the þing-01 redaction clause is **already mechanically enforced** (`tests/test_redaction_guard.py` — the delivery modules stay credential-blind…)"* — and **Cluster I's fix strategy cites this very file** as an example of the guard pattern working. Both were written in good faith and both are now overclaims. That is the cluster's disease reproducing inside the cluster's own prescription.

It is also the **sixth** instance of the stale-claim class found in two days, after C-63 and C-47 (filed Open with the defect fixed), C-43/C-59/C-61 (filed Open with their stated closing conditions met), #158 (closed with the code half undone) and `tests/test_validation.py` (claiming fidelity to a method it no longer resembles — **C-03**). The common shape is not carelessness: it is that a *move* leaves prose and paths behind, and nothing in this repo asserts that a declared path exists.

**Mitigation:** re-point the four roots at `contract/`, and add a root-existence assertion so a future relocation fails loudly instead of silently narrowing. The second half is the load-bearing one — re-pointing fixes today, asserting existence fixes the class. Tracked as a story under **epic #181**.

Cross-refs: **C-57** (whose "already mechanically enforced" claim this falsifies — corrected in place), **C-03** (the same class in `test_validation.py`), **C-46** (a different guard that also does not run, by a different mechanism), **C-63**, **C-47**, **Cluster I**, #135, #153, #182.

---

### C-46: `test_datafactory_deploy_readiness` is hardcoded to a local path — CI-skipped, and currently failing on the one machine that runs it — RESOLVED

| Field | Value |
Expand Down Expand Up @@ -716,7 +726,7 @@ Two named changes are already anticipated and will fire this trigger: the **reti

**Deliberately out of scope here:** the þing-01 redaction clause is mechanically enforced (`tests/test_redaction_guard.py` — the delivery modules stay credential-blind and the provenance description is a closed keyset), and D2's ruling that **integration tests against the production Appwrite project are FORBIDDEN** (no non-production project exists) is a standing prohibition, not a drift risk.

**⚠ CORRECTED 2026-08-02.** The word *already* above overclaimed: **C-74** showed that guard scans one of its five declared roots, four having pointed at paths that #153 moved. The keyset half is enforced; the credential-blindness half is enforced over `delivery/` only until C-74 lands.
**⚠ CORRECTED 2026-08-02, then restored the same day.** The word *already* above overclaimed at the time: **C-74** showed that guard scanning one of its five declared roots, four having pointed at paths #153 moved. **C-74 closed later that day (S10 / #192)** — the roots are re-pointed, the scan covers 17 files, and a declared root that does not exist now fails rather than emptying the scan silently. The sentence above is true again, and the episode is left visible because a claim that was false for two days is worth more as a record than as a correction quietly reverted.

Cross-refs: C-74 (the guard this paragraph vouched for), C-33 (store identity still hardcoded per store — the same env surface, different concern), C-58 (what happens when a coordinate is wrong rather than missing), C-44 (the pipeline-core version coupling that would carry a registry change), issues #134/#135/#138 (this repo's discharged þing-01 obligations), #104 (README env block placeholders).

Expand Down
94 changes: 87 additions & 7 deletions tests/test_redaction_guard.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,27 @@
# the environment and never see a key (the manager's DIP port hands them data
# only). A hit here means a credential VALUE could newly reach a delivery log
# line or artifact — re-run the #135 audit before allowing it.
#
# **These paths moved and this list did not** (register C-74, S10 / #192). Epic
# #148's #153 relocated the machinery from ``unfao/`` to ``contract/``; four of the
# five roots below kept pointing at ``unfao/`` and the guard went on passing while
# scanning **6 files instead of 16**. It was not wrong; it was not updated.
#
# Written out as an explicit list rather than derived from the package tree —
# deliberately. ``tests/test_clone_readiness.py`` enumerates an overlapping set for
# a *different* question ("does the machinery import without the partner?"), and
# folding them together would couple two guards whose sets are free to diverge:
# a new module can be partner-neutral without being credential-blind. Two lists
# that each say what they mean beat one that means neither.
#
# What makes the explicit list safe is the root-existence assertion below. That is
# the half that fixes the *class* rather than today's instance.
_CREDENTIAL_BLIND = [
_PKG / "unfao" / "wire",
_PKG / "contract" / "wire",
_PKG / "delivery",
_PKG / "unfao" / "historical.py",
_PKG / "unfao" / "track_a_source.py",
_PKG / "unfao" / "frame_extraction.py",
_PKG / "contract" / "historical.py",
_PKG / "contract" / "track_a_source.py",
_PKG / "contract" / "frame_extraction.py",
]

_FORBIDDEN_TOKENS = ("os.environ", "getenv", "load_dotenv", "API_KEY", "credentials")
Expand All @@ -38,20 +53,85 @@ def _python_sources(path: Path):
yield from sorted(path.rglob("*.py"))


def test_wire_and_delivery_modules_stay_credential_blind():
def _scan(roots: list[Path], base: Path = _PKG) -> list[str]:
"""Every forbidden-token hit under ``roots``, reported relative to ``base``.

``base`` is a parameter only so the guard can be pointed at a synthetic tree and
proven to bite — see ``test_the_guard_would_catch_a_credential_reference``.
"""
hits = []
for root in _CREDENTIAL_BLIND:
for root in roots:
for source in _python_sources(root):
text = source.read_text()
for token in _FORBIDDEN_TOKENS:
if token in text:
hits.append(f"{source.relative_to(_PKG)}: {token!r}")
hits.append(f"{source.relative_to(base)}: {token!r}")
return hits


def test_every_declared_credential_blind_root_exists():
"""The half that fixes the class, not the instance (register C-74).

``_python_sources`` calls ``rglob`` on a directory that is not a file — and
``rglob`` on a **nonexistent** directory yields an empty iterator rather than
raising. So a missing root and a clean root are indistinguishable to the scan,
and a package move silently narrows it to nothing while the suite stays green.

That is precisely what happened between #153 and this test: four of five roots
resolved to nothing for two days, and the þing-01 #135 audit was pinned over
roughly a third of what it certified.
"""
missing = [str(r.relative_to(_PKG)) for r in _CREDENTIAL_BLIND if not r.exists()]
assert not missing, (
f"declared credential-blind root(s) no longer exist: {missing}. A move that "
"leaves this list behind does not fail the scan — it empties it. Re-point the "
"list at where the modules live now (þing-01 #135, register C-74)."
)


def test_wire_and_delivery_modules_stay_credential_blind():
hits = _scan(_CREDENTIAL_BLIND)
assert not hits, (
"credential-blind delivery modules now reference the environment or a "
f"credential (þing-01 #135 audit invalidated): {hits}"
)


def test_the_scan_covers_the_whole_audited_surface():
"""A count, so silent narrowing shows up as a number rather than as silence.

The #135 audit certified the delivery path — the wire, the invariants, and the
three seam modules. It is ~16 files; it was 6 while the roots were stale. The
bound is deliberately loose (a lower bound, not an equality) so adding a module
does not fail the suite, while *losing* most of them does.
"""
scanned = sum(1 for root in _CREDENTIAL_BLIND for _ in _python_sources(root))
assert scanned >= 15, (
f"the credential-blind scan covers only {scanned} files. The audited surface "
"is the wire (8), delivery (6) and three seam modules. A number well below "
"that means roots are resolving to nothing again — see C-74."
)


def test_the_guard_would_catch_a_credential_reference(tmp_path):
"""The defect it was written for: a delivery module reading the environment.

A guard nobody has watched fail is a guard nobody knows the shape of. This one
was passing for two days over four roots that resolved to nothing (C-74), which
is exactly what an unproven guard buys you.
"""
(tmp_path / "clean.py").write_text("def build(table):\n return table\n")
assert _scan([tmp_path], base=tmp_path) == [], "a clean module was flagged"

(tmp_path / "leaky.py").write_text(
"import os\nkey = os.environ['APPWRITE_DATASTORE_API_KEY']\n"
)
hits = _scan([tmp_path], base=tmp_path)
assert any("leaky.py" in h for h in hits), (
f"a module reading a credential from the environment was not flagged: {hits}"
)


def test_provenance_carries_only_the_declared_closed_keyset():
# The description is uploaded into the store's metadata — it leaves the
# machine. Its content must stay exactly the declared delivery primitives.
Expand Down
Loading